Re: [PATCH 2/2] t: add tests for safe.directory when running with sudo

[Junio C Hamano <gitster@pobox.com>]

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> In a previous commit the functionality for fixing this regression was
> implemented, so add a test for it and the relevant infrastructure.
>
> This new test file is meant to be run in CI and checks for a SUDO
> prerequisite which requires a passwordless configuration if run
> locally, but that is common there.
>
> It could be run locally by first running sudo, so the credentials are
> temporarily cached and then running it as :
>
>   $ IKNOWWHATIAMDOING=YES ./t0034-root-safe-directory.sh
>
> It is slightly ackward as it needs to clean up after itself since the

"awkward", I think.

> test framework would fail to do its own cleanup otherwise, and for
> simplicity creates its own subtree while ignoring the one provided by
> the framework, but doing some better integration has been punted.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  t/t0034-root-safe-directory.sh | 51 ++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100755 t/t0034-root-safe-directory.sh
>
> diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh
> new file mode 100755
> index 00000000000..c62bf3777c0
> --- /dev/null
> +++ b/t/t0034-root-safe-directory.sh
> @@ -0,0 +1,51 @@
> +#!/bin/sh
> +
> +test_description='verify safe.directory checks while running as root'
> +
> +. ./test-lib.sh
> +
> +if [ "$IKNOWWHATIAMDOING" != "YES" ]; then
> +	skip_all="You must set env var IKNOWWHATIAMDOING=YES in order to run thi
> +s test"
> +	test_done
> +fi
> +
> +if ! test_have_prereq NOT_ROOT
> +then
> +	skip_all="this test uses sudo to run as root"

As a statement of fact, there is nothing wrong per-se in this
message.  It may be an explanation why you should not run this as
root, but as an explanation of the reason why all tests in this
script are skipped, and more importantly, hint to testers what they
need to do to correct the situation, this is too cryptic to readers.

Perhaps telling them

	skip_all="do not run this test as root"

is more important and useful.

> +	test_done
> +fi
> +
> +doalarm () {
> +	perl -e 'alarm shift; exec @ARGV' -- "$@"
> +}
> +
> +test_lazy_prereq SUDO '
> +	doalarm 1 sudo id -u >u &&
> +	id -u root >r &&
> +	test_cmp u r
> +'
> +
> +test_expect_success SUDO 'setup' '
> +	sudo rm -rf root &&
> +	mkdir -p root/r &&
> +	sudo chown root root &&
> +	(
> +		cd root/r &&
> +		git init
> +	)
> +'
> +
> +test_expect_success SUDO 'sudo git status works' '
> +	(
> +		cd root/r &&
> +		git status &&
> +		sudo git status
> +	)
> +'

All of the above are positive tests.  One possible negative test is
to prepare a root-owned repository and see what "git status" as
yourself in there does.  And also see what "git status" as root
there does.

test_expect_success SUDO 'in root owned repository' '
	mkdir root/p
        sudo chown root root/p &&
	sudo git init root/p &&

	# owned by other person (root), do I see it as a repository?
	(
		cd root/p &&
		test_must_fail git status
	) &&

	# owned by root, can I access it under sudo?
	(
		cd root/p &&
		sudo git status
	)
'

> +test_expect_success SUDO 'cleanup' '
> +	sudo rm -rf root
> +'
> +
> +test_done