Re: [PATCH v6] Add an explicit GIT_DIR to the list of excludes

2014-06-13 Thread Pasha Bolokhov
 + 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

2014-06-13 Thread Junio C Hamano
Pasha Bolokhov pasha.bolok...@gmail.com 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 .number.  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

2014-06-12 Thread Johannes Sixt
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
 + test_cmp status.actual.6 status.expect.6
 + )
 +'
 +
 +test_done
 

--
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