Re: [GSoC][PATCH v7 04/26] stash: renamed test cases to be more descriptive
From: Thomas Gummerer <hidden>
Date: 2018-08-15 19:31:39
Subject: Re: [GSoC][PATCH v7 04/26] stash: renamed test cases to be more descriptive
Please use the imperative mood in the title and the commit messages
themselves. From Documentation/SubmittingPatches:
Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behavior.
From a quick skim over the rest of the series, this also applies to
some of the subsequent patches in the series.
On 08/08, Paul-Sebastian Ungureanu wrote:quoted hunk ↗ jump to hunk
Renamed some test cases' labels to be more descriptive and under 80 characters per line. Signed-off-by: Paul-Sebastian Ungureanu <redacted> --- t/t3903-stash.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index de6cab1fe..8d002a7f2 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh@@ -604,7 +604,7 @@ test_expect_success 'stash show -p - no stashes on stack, stash-like argument' ' test_cmp expected actual ' -test_expect_success 'stash drop - fail early if specified stash is not a stash reference' ' +test_expect_success 'drop: fail early if specified stash is not a stash ref' ' git stash clear && test_when_finished "git reset --hard HEAD && git stash clear" && git reset --hard &&@@ -618,7 +618,7 @@ test_expect_success 'stash drop - fail early if specified stash is not a stash r git reset --hard HEAD ' -test_expect_success 'stash pop - fail early if specified stash is not a stash reference' ' +test_expect_success 'pop: fail early if specified stash is not a stash ref' ' git stash clear && test_when_finished "git reset --hard HEAD && git stash clear" && git reset --hard &&@@ -682,7 +682,7 @@ test_expect_success 'invalid ref of the form "n", n >= N' ' git stash drop ' -test_expect_success 'stash branch should not drop the stash if the branch exists' ' +test_expect_success 'branch: should not drop the stash if the branch exists' '
Since we're adjusting the titles of the tests here I'll allow myself to nitpick a little :) Maybe "branch: do not drop the stash if the branch exists", which sounds more like an assertion, as the "pop" and "drop" titles above.
quoted hunk ↗ jump to hunk
git stash clear && echo foo >file && git add file &&@@ -693,7 +693,7 @@ test_expect_success 'stash branch should not drop the stash if the branch exists git rev-parse stash@{0} -- ' -test_expect_success 'stash branch should not drop the stash if the apply fails' ' +test_expect_success 'branch: should not drop the stash if the apply fails' ' git stash clear && git reset HEAD~1 --hard && echo foo >file &&@@ -707,7 +707,7 @@ test_expect_success 'stash branch should not drop the stash if the apply fails' git rev-parse stash@{0} -- ' -test_expect_success 'stash apply shows status same as git status (relative to current directory)' ' +test_expect_success 'apply: shows same status as git status (relative to ./)' '
s/shows/show/ above maybe? This used to be a full sentence previously, where 'shows' was appropriate, but I think "show" sounds better after the colon.
quoted hunk ↗ jump to hunk
git stash clear && echo 1 >subdir/subfile1 && echo 2 >subdir/subfile2 &&@@ -1048,7 +1048,7 @@ test_expect_success 'stash push -p with pathspec shows no changes only once' ' test_i18ncmp expect actual ' -test_expect_success 'stash push with pathspec shows no changes when there are none' ' +test_expect_success 'push: <pathspec> shows no changes when there are none' '
Maybe "push <pathspec>: show no changes when there are none"? "push <pathspec>" would be the rest of the 'git stash' command, having the colon in between them seems a bit odd.
quoted hunk ↗ jump to hunk
>foo && git add foo && git commit -m "tmp" &&@@ -1058,7 +1058,7 @@ test_expect_success 'stash push with pathspec shows no changes when there are no test_i18ncmp expect actual ' -test_expect_success 'stash push with pathspec not in the repository errors out' ' +test_expect_success 'push: <pathspec> not in the repository errors out' '
This one makes sense to me.
>untracked && test_must_fail git stash push untracked && test_path_is_file untracked -- 2.18.0.573.g56500d98f