Re: [PATCH v2 1/2] modernize t7300
Eric Sunshine writes: > Alternately, update test_path_foo() functions to accept multiple > pathnames, or is that too ugly? That actually would have been my first choice, except (1) that path_is_missing has a cruft whose usefulness is dubious, and (2) that "path_exists A B C" and "path_is_missing D E F" would be gramatically incorrect. I think we should first see if we can remove that "customized message" that appears only in path_is_missing and remove it if we can. Extending "path_is_missing A B C" and friends to work would then become trivial. Thanks. -- 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 v2 1/2] modernize t7300
On Mon, Dec 7, 2015 at 4:40 PM, Junio C Hamano wrote: > James writes: >> @@ -46,15 +46,15 @@ test_expect_success 'git clean' ' >> mkdir -p build docs && >> touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && >> git clean && >> - test -f Makefile && >> - test -f README && >> - test -f src/part1.c && >> - test -f src/part2.c && >> - test ! -f a.out && >> - test ! -f src/part3.c && >> - test -f docs/manual.txt && >> - test -f obj.o && >> - test -f build/lib.so >> + test_path_is_file Makefile && >> + test_path_is_file README && >> + test_path_is_file src/part1.c && >> + test_path_is_file src/part2.c && >> + test_path_is_missing a.out && >> + test_path_is_missing src/part3.c && >> + test_path_is_file docs/manual.txt && >> + test_path_is_file obj.o && >> + test_path_is_file build/lib.so > > The verbosity of this conversion makes me wonder if we want to have > "test_paths_are_files" and "test_paths_are_missing". For that > matter, this test does not really care about the distinction between > files and directories (e.g. some tests said "test ! -d docs" and > would have passed if there were a 'docs' regular file, but what we > really care about is the path 'docs' is _gone_), so what we want may > be test_paths_exist and test_paths_are_missing. With that, the > above hunk would become > > test_paths_exist Makefile README src/part1.c src/part2.c \ > obj.o build/lib.so && > test_paths_are_missing a.out src/part3.c > > I dunno. Alternately, update test_path_foo() functions to accept multiple pathnames, or is that too ugly? -- 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 v2 1/2] modernize t7300
On Sun, Dec 6, 2015 at 9:58 AM, James wrote: > From: James Rouzier This would be a good place to explain how you are modernizing the test script. Right now, you're just updating to take advantage of test_path_is_foo(), but some other modernizations you could do include: * using '>' rather than 'touch' to create empty files when the timestamp doesn't matter (which is all cases in this script) * (optional) replace unnecessarily complex 'mkdir -p foo' with simpler 'mkdir foo' (but leave "mkdir -p foo/bar" as is) * drop blank lines before and after test body; for instance: test_expect_success 'foo' ' blah && bloo ' becomes: test_expect_success 'foo' ' blah && bloo ' > --- > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > @@ -609,32 +609,30 @@ test_expect_success 'force removal of nested git work > tree' ' > test_expect_success 'git clean -e' ' > rm -fr repo && > mkdir repo && > - ( > - cd repo && > - git init && > - touch known 1 2 3 && > - git add known && > - git clean -f -e 1 -e 2 && > - test -e 1 && > - test -e 2 && > - ! (test -e 3) && > - test -e known > - ) > + cd repo && The previous working directory is not automatically restored at the end of the test, so this "cd" without corresponding "cd .." causes following tests to execute at the incorrect location. Unfortunately, you can't just place "cd .." at the end of a test since it will be skipped if something before the "cd .." fails. The way the existing code deals with this is by using a subshell: mkdir repo && ( cd repo && ... ) The "cd repo" is inside the subshell, so it doesn't affect the parent shell, and when the subshell exits, the parent shell remains at the correct working directory (regardless of whether the test succeeded or failed). Therefore, you don't want to drop the subshell. > + git init && > + touch known 1 2 3 && > + git add known && > + git clean -f -e 1 -e 2 && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > ' -- 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 v2 1/2] modernize t7300
James writes: > From: James Rouzier > > --- Missing sign-off. > t/t7300-clean.sh | 382 > +++ > 1 file changed, 190 insertions(+), 192 deletions(-) > > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > index 86ceb38..d555bb6 100755 > --- a/t/t7300-clean.sh > +++ b/t/t7300-clean.sh > @@ -28,15 +28,15 @@ test_expect_success 'git clean with skip-worktree > .gitignore' ' > mkdir -p build docs && > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > git clean && > - test -f Makefile && > - test -f README && > - test -f src/part1.c && > - test -f src/part2.c && > - test ! -f a.out && > - test ! -f src/part3.c && > - test -f docs/manual.txt && > - test -f obj.o && > - test -f build/lib.so && > + test_path_is_file Makefile && > + test_path_is_file README && > + test_path_is_file src/part1.c && > + test_path_is_file src/part2.c && OK. > + test_path_is_missing a.out && > + test_path_is_missing src/part3.c && This is better than the original, which may have said "OK" upon seeing a directory called "a.out". > + test_path_is_file docs/manual.txt && > + test_path_is_file obj.o && > + test_path_is_file build/lib.so && OK. > git update-index --no-skip-worktree .gitignore && > git checkout .gitignore > ' > @@ -46,15 +46,15 @@ test_expect_success 'git clean' ' > mkdir -p build docs && > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > git clean && > - test -f Makefile && > - test -f README && > - test -f src/part1.c && > - test -f src/part2.c && > - test ! -f a.out && > - test ! -f src/part3.c && > - test -f docs/manual.txt && > - test -f obj.o && > - test -f build/lib.so > + test_path_is_file Makefile && > + test_path_is_file README && > + test_path_is_file src/part1.c && > + test_path_is_file src/part2.c && > + test_path_is_missing a.out && > + test_path_is_missing src/part3.c && > + test_path_is_file docs/manual.txt && > + test_path_is_file obj.o && > + test_path_is_file build/lib.so The verbosity of this conversion makes me wonder if we want to have "test_paths_are_files" and "test_paths_are_missing". For that matter, this test does not really care about the distinction between files and directories (e.g. some tests said "test ! -d docs" and would have passed if there were a 'docs' regular file, but what we really care about is the path 'docs' is _gone_), so what we want may be test_paths_exist and test_paths_are_missing. With that, the above hunk would become test_paths_exist Makefile README src/part1.c src/part2.c \ obj.o build/lib.so && test_paths_are_missing a.out src/part3.c I dunno. > test_expect_success 'git clean -e' ' > rm -fr repo && > mkdir repo && > - ( > - cd repo && > - git init && > - touch known 1 2 3 && > - git add known && > - git clean -f -e 1 -e 2 && > - test -e 1 && > - test -e 2 && > - ! (test -e 3) && > - test -e known > - ) > + cd repo && > + git init && > + touch known 1 2 3 && > + git add known && > + git clean -f -e 1 -e 2 && > + test_path_is_file 1 && > + test_path_is_file 2 && > + test_path_is_missing 3 && > + test_path_is_file known > ' I think this is wrong. The next test piece will be run inside "repo" directory with this patch applied. Don't lose the subshell here. -- 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
[PATCH v2 1/2] modernize t7300
From: James Rouzier --- t/t7300-clean.sh | 382 +++ 1 file changed, 190 insertions(+), 192 deletions(-) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 86ceb38..d555bb6 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -28,15 +28,15 @@ test_expect_success 'git clean with skip-worktree .gitignore' ' mkdir -p build docs && touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && git clean && - test -f Makefile && - test -f README && - test -f src/part1.c && - test -f src/part2.c && - test ! -f a.out && - test ! -f src/part3.c && - test -f docs/manual.txt && - test -f obj.o && - test -f build/lib.so && + test_path_is_file Makefile && + test_path_is_file README && + test_path_is_file src/part1.c && + test_path_is_file src/part2.c && + test_path_is_missing a.out && + test_path_is_missing src/part3.c && + test_path_is_file docs/manual.txt && + test_path_is_file obj.o && + test_path_is_file build/lib.so && git update-index --no-skip-worktree .gitignore && git checkout .gitignore ' @@ -46,15 +46,15 @@ test_expect_success 'git clean' ' mkdir -p build docs && touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && git clean && - test -f Makefile && - test -f README && - test -f src/part1.c && - test -f src/part2.c && - test ! -f a.out && - test ! -f src/part3.c && - test -f docs/manual.txt && - test -f obj.o && - test -f build/lib.so + test_path_is_file Makefile && + test_path_is_file README && + test_path_is_file src/part1.c && + test_path_is_file src/part2.c && + test_path_is_missing a.out && + test_path_is_missing src/part3.c && + test_path_is_file docs/manual.txt && + test_path_is_file obj.o && + test_path_is_file build/lib.so ' @@ -63,15 +63,15 @@ test_expect_success 'git clean src/' ' mkdir -p build docs && touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && git clean src/ && - test -f Makefile && - test -f README && - test -f src/part1.c && - test -f src/part2.c && - test -f a.out && - test ! -f src/part3.c && - test -f docs/manual.txt && - test -f obj.o && - test -f build/lib.so + test_path_is_file Makefile && + test_path_is_file README && + test_path_is_file src/part1.c && + test_path_is_file src/part2.c && + test_path_is_file a.out && + test_path_is_missing src/part3.c && + test_path_is_file docs/manual.txt && + test_path_is_file obj.o && + test_path_is_file build/lib.so ' @@ -80,15 +80,15 @@ test_expect_success 'git clean src/ src/' ' mkdir -p build docs && touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && git clean src/ src/ && - test -f Makefile && - test -f README && - test -f src/part1.c && - test -f src/part2.c && - test -f a.out && - test ! -f src/part3.c && - test -f docs/manual.txt && - test -f obj.o && - test -f build/lib.so + test_path_is_file Makefile && + test_path_is_file README && + test_path_is_file src/part1.c && + test_path_is_file src/part2.c && + test_path_is_file a.out && + test_path_is_missing src/part3.c && + test_path_is_file docs/manual.txt && + test_path_is_file obj.o && + test_path_is_file build/lib.so ' @@ -97,16 +97,16 @@ test_expect_success 'git clean with prefix' ' mkdir -p build docs src/test && touch a.out src/part3.c docs/manual.txt obj.o build/lib.so src/test/1.c && (cd src/ && git clean) && - test -f Makefile && - test -f README && - test -f src/part1.c && - test -f src/part2.c && - test -f a.out && - test ! -f src/part3.c && - test -f src/test/1.c && - test -f docs/manual.txt && - test -f obj.o && - test -f build/lib.so + test_path_is_file Makefile && + test_path_is_file README && + test_path_is_file src/part1.c && + test_path_is_file src/part2.c && + test_path_is_file a.out && + test_path_is_missing src/part3.c && + test_path_is_file src/test/1.c && + test_path_is_file docs/manual.txt && + test_path_is_file obj.o && + test_path_is_file build/lib.so ' @@ -160,16 +160,16 @@ test_expect_success 'git clean -d with prefix and path' ' mkdir -p build docs src/feature && touch a.out src/part3.c src/feature/file.c docs/manual.txt obj.o build/lib.so && (cd src/ && git clean -d feature/) && - test -f Makefile && - test -f README && - test -f src/part1.c && - test -f src/part2.c && - test -f a.out && - test -f src/part3.c &