Re: [PATCH v6] Add an explicit GIT_DIR to the list of excludes
Pasha Bolokhov writes:
>>> + test_cmp status.actual.2 status.expect.2
>>
>> It is customary to call the files 'expect' and 'actual'. Furthermore,
>> swap the order so that in case of a failure the diff shows how the
>> actual text was changed from the expected text:
>>
>> test_cmp status.expect.2 status.actual.2
>
> So, is naming the files "status.expect.2" instead of just
> "expect"/"actual" ok or not?
Unless there is a compelling reason not to, just use expect/actual,
without "status." or ".". When debugging, people pass the
"-i" (and often "-v") to the script which would stop at the first
breakage. If they can inspect the result in "actual", that is far
more pleasant than having to know that it is status.actual.43 they
have to inspect while ignoring clutters in status.actual.{1-42} that
have nothing to do with the breakage they are dealing with.
> Ok, no problem, will change that. The only thing, I saw this in other
> tests, so decided to use it too. Those tests use "test_mighf_fail rm"
"might_fail" and "must_fail" are totally different beasts.
"must_fail" is about controlled failure exit (i.e. no segfaulting
allowed) and we avoid using it on non-git things.
When you run clean-up commands without knowing if there is anythning
to clean-up, e.g. you may or may not have successfully created a
junk file and you want to make sure that file does not exist. It
happens that "rm junk" has a convenient option "-f" to make it not
barf upon missing "junk", so you can do so with "rm -f junk" instead
of saying "test_might_fail rm junk", but "might_fail" is designed to
be useful for commands that do not have such a convenient option.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add an explicit GIT_DIR to the list of excludes
>> + test_cmp status.actual.2 status.expect.2 > > It is customary to call the files 'expect' and 'actual'. Furthermore, > swap the order so that in case of a failure the diff shows how the > actual text was changed from the expected text: > > test_cmp status.expect.2 status.actual.2 So, is naming the files "status.expect.2" instead of just "expect"/"actual" ok or not? Those prefixes "status" etc just help sorting out where the problem lies that causes the test to fail. But let me know if this is too "detailed" > > Moreover, test_*_fail helpers are intended to be used only with git > commands; we don't expect system commands to fail in unexpected ways. Ok, no problem, will change that. The only thing, I saw this in other tests, so decided to use it too. Those tests use "test_mighf_fail rm" and "ls" seemingly without invocation of git -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add an explicit GIT_DIR to the list of excludes
Am 12.06.2014 01:28, schrieb Pasha Bolokhov: > +test_expect_success "setup" ' > + mkdir repo-inside/ && > + ( > + cd repo-inside/ && > + for f in a b c d > + do > + echo "DATA" >"$f" || exit 1 > + done && > + mkdir dir1 dir1/meta && > + mkdir dir1/ssubdir dir1/ssubdir/meta && > + for f in e f g h > + do > + echo "MORE DATA" >"dir1/$f" || exit 1 > + done && > + echo "EVEN more Data" >dir1/meta/aa && > + echo "Data and BAIT" >dir1/ssubdir/meta/aaa && > + mkdir dir2 > + echo "Not a Metadata File" >dir2/meta &&-chain broken twice. We already use 'mkdir -p' elsewhere; you can use it here as well to contract several mkdir invocations. > + git --git-dir=meta init > + ) && > + mkdir repo-outside/ repo-outside/external repo-outside/tree && > + ( > + cd repo-outside/tree && > + for f in n o p q > + do > + echo "Literal Data" >"$f" || exit 1 > + done && > + mkdir meta sub sub/meta && > + echo "Sample data" >meta/bb && > + echo "Stream of data" >sub/meta/bbb && > + git --git-dir=../external/meta init > + ) > +' > + > + > +# > +# The first set of tests (the repository is inside the work tree) > +# > +test_expect_success "'git status' ignores the repository directory" ' > + ( > + cd repo-inside && > + git --git-dir=meta --work-tree=. status --porcelain > --untracked=all | > + grep meta | sort >status.actual.2 && Please do not place a git invocation in a pipline such that it is not the last command; its exist status would be ignored: git --git-dir=meta --work-tree=. status --porcelain --untracked=all >status.actual.2+ && grep meta status.actual.2+ | sort >status.actual.2 && There are more cases like this later in the patch. > + cat >status.expect.2 <<-\EOF && > + ?? dir1/meta/aa > + ?? dir1/ssubdir/meta/aaa > + ?? dir2/meta > + EOF > + test_cmp status.actual.2 status.expect.2 It is customary to call the files 'expect' and 'actual'. Furthermore, swap the order so that in case of a failure the diff shows how the actual text was changed from the expected text: test_cmp status.expect.2 status.actual.2 > + ) > +' > + > +test_expect_success "'git add -A' ignores the repository directory" ' > + ( > + cd repo-inside && > + git --git-dir=meta --work-tree=. add -A && > + git --git-dir=meta --work-tree=. status --porcelain | grep meta > | sort >status.actual.3 && > + cat >status.expect.3 <<-\EOF && > + A dir1/meta/aa > + A dir1/ssubdir/meta/aaa > + A dir2/meta > + EOF > + test_cmp status.actual.3 status.expect.3 > + ) > +' > + > +test_expect_success "'git grep --exclude-standard' ignores the repository > directory" ' > + ( > + cd repo-inside && > + test_might_fail git --git-dir=meta \ > + grep --no-index --exclude-standard BAIT >grep.actual.4 > && > + cat >grep.expect.4 <<-\EOF && > + dir1/ssubdir/meta/aaa:Data and BAIT > + EOF > + test_cmp grep.actual.4 grep.expect.4 > + ) > +' > + > +# > +# The second set of tests (the repository is outside of the work tree) > +# > +test_expect_success "'git status' acknowledges directories 'meta' \ > +if repo is not within work tree" ' > + test_might_fail rm -rf meta/ && How might this fail? Only if permissions are wrong, and then we do want this to fail. Moreover, test_*_fail helpers are intended to be used only with git commands; we don't expect system commands to fail in unexpected ways. > + ( > + cd repo-outside/tree && > + git --git-dir=../external/meta init && > + git --git-dir=../external/meta --work-tree=. status --porcelain > --untracked=all | > + grep meta | sort >status.actual.5 && > + cat >status.expect.5 <<-\EOF && > + ?? meta/bb > + ?? sub/meta/bbb > + EOF > + test_cmp status.actual.5 status.expect.5 > + ) > +' > + > +test_expect_success "'git add -A' adds 'meta' if the repo is outside the > work tree" ' > + ( > + cd repo-outside/tree && > + git --git-dir=../external/meta --work-tree=. add -A && > + git --git-dir=../external/meta --work-tree=. status --porcelain > --untracked=all | > + grep meta | sort >status.actual.6 && > + cat >status.expect.6 <<-\EOF && > + A meta/bb > + A sub/meta/bbb > + EOF > +
