On Tue, Oct 8, 2024 at 2:48 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Samuel Adekunle Abraham via GitGitGadget" <gitgitgadget@xxxxxxxxx> > writes: > > > From: Abraham Samuel Adekunle <abrahamadekunle50@xxxxxxxxx> > > > Manual validation is still needed for "test ! -f foo". If the > original expects 'foo' to be gone, and has a reason to expect 'foo' > to be a file when the code being tested is broken, then rewriting it > into "test_path_is_missing" is OK. But otherwise, the original > would have been happy when 'foo' existed as a directory and > rewriting it into "test_path_is_missing" is not quite right. > > This check cannot be done mechanically, unfortunately. Hits from > > $ git show | grep -e 'test ! -[df]' > > need to be eyeballed to make sure that it is reasonable to rewrite > "test ! -f foo" into "test_path_is_missing foo". > > For example: > > > mkdir -p build docs && > > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > > git clean && > > ... > > - test ! -f a.out && > > - test ! -f src/part3.c && > > this test creates a.out and src/part3.c as regular files before > running "git clean", so the expected failure modes do not include > a.out to be a directory (which would also make "test ! -f a.out" > happy), and rewriting it to "test_path_is_missing a.out" is fine. > Hi Junio, Thanks again for your time and review. I have gone through all the instances of "test ! - [df]" and for each test case where "test ! -f foo" was used, foo was first created as a regular file in the control flow before "git clean" was called and is expected not to exist afterwards. a few more examples are to the one you referenced above are shown below; > 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 && and > mkdir -p build docs && > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > git clean -d -X -e 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 && Also for the tests where "test ! -d foo" was used, the control flow followed similar pattern as mentioned above where foo was created as a directory and then "git clean -d" was called. The control flow expected foo to be a directory which could have been removed afterwards as can be seen below. > @@ -300,15 +300,15 @@ test_expect_success 'git clean -d -x with ignored tracked directory' ' > mkdir -p build docs && > touch a.out src/part3.c docs/manual.txt obj.o build/lib.so && > git clean -d -x -e 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 ! -d docs && > - test ! -f obj.o && > - test ! -d build and > test_expect_success 'should clean things that almost look like git but are not' ' > @@ -624,9 +624,9 @@ test_expect_success 'force removal of nested git work tree' ' > test_commit deeply.nested deeper.world > ) && > git clean -f -f -d && > - ! test -d foo && > - ! test -d bar && > - ! test -d baz This was the reason for replacing "test ! -[df]" with "test_path_is_missing" where I think is appropriate. I will appreciate it and be very grateful if test instances in this script where "test_path_is_missing" is inappropriate to be used are pointed out by other maintainers as well. Thanks once again for your time.
