Re: [GSoC] [PATCH] t1011: replace test -f with test_path_is_file

Siddharth Asthana <siddharthasthana31@gmail.com> writes:

missing Sign-off.

Given the definition of the helper function, i.e.

        test_path_is_file () {
                test "$#" -ne 1 && BUG "1 param"
                if ! test -f "$1"
                then
                        echo "File $1 doesn't exist"
                        false
                fi
        }

the new test will _complain_ "init.t doesn't exist" when we have successfully run the test, while it will be _silent_ when init.t that _should_ not exist is there.

Which is the complete opposite of the spirit of why we want to use the helper when we expect the path "$1" to exist, i.e. loudly fail when our expectation is _not_ met.

$ git grep '! test_path_is' t/

shows that we already have such a misuse of test_path_is_dir in one place, but luckily we do not have any for test_path_is_file or other similar helpers. test_path_is_hidden is sort-of OK as that is not about verbosity.

In these two test, we do not expect init.t or sub/added to _exist_ at all. It's not like we are happy if we see init.d exist as a directory (which is not a file). test_path_is_missing is probably the right helper to use.

It is not very plausible that we'd want to assert that existence of a path as a file the only bad condition (i.e. we are happy if the path did not exist or it is a directory, symlink, or a socket), so I think the simple

	Never use '! test_path_is_file'; test_path_is_missing may be
	what you are looking for.

is a good enough rule.

If not, we could allow the caller to write such a convoluted "only existence of a path as a file is unacceptable and everything else is good" assertion as

    test_path_is_file ! init.d

with something like

        test_path_is_file () {
		expecting_file=true
		if test "$1" = "!"
		then
			expecting_file=false
			shift
		fi
                test "$#" -ne 1 && BUG "1 param"
                if test -f "$1"
                then
                	$expecting_file || echo "File $1 exists"
                        $expecting_file
		else
			$expecting_file && echo "File $1 doesn't exist"
                        ! $expecting_file
                fi
        }

but I do not think we want to go that way.