Re: [PATCH 3/3] t7900: make macOS-specific test work on Windows

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 27, 2020, 15:05 UTC
Message-ID: <87o8jikfh7.fsf@evledraar.gmail.com>
In-Reply-To: <20201127075054.31174-4-sunshine@sunshineco.com>

On Fri, Nov 27 2020, Eric Sunshine wrote:

Show 32 quoted lines

> Although `git maintenance start` and `git maintenance stop` necessarily
> invoke platform-specific scheduling utilities, their related tests have
> been carefully crafted -- with one minor exception -- to work correctly
> on any platform, thus improving overall coverage. The exception is that
> the macOS-specific test fails on Windows due to unportable use of
> `$(id -u)` and comparison involving the value of $HOME which suffers
> from the typical shortcoming on that platform in which the same path may
> be represented two different ways depending upon its source (i.e. as a
> Windows path `C:/git-sdk-64/usr/src/git/foo` versus as a Unix path
> `/usr/src/git/foo`). Fix both problems and drop the !MINGW prerequisite
> from the macOS-specific test, thus allowing the test to run on Windows,
> as well.
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t7900-maintenance.sh | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index ef3aec3253..500eaae4fd 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -408,8 +408,10 @@ test_expect_success 'start preserves existing schedule' '
>  	grep "Important information!" cron.txt
>  '
>  
> -test_expect_success !MINGW 'start and stop macOS maintenance' '
> -	uid=$(id -u) &&
> +test_expect_success 'start and stop macOS maintenance' '
> +	uid=$(test-tool getuid) &&
> +	# ensure $HOME can be compared against hook arguments on all platforms
> +	pfx=$(cd "$HOME" && pwd) &&

This seems equally portable, and means your 2/3 isn't needed, no?
    
    diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
    index c3dcb9cb4d..b23f77aebc 100755
    --- a/t/t7900-maintenance.sh
    +++ b/t/t7900-maintenance.sh
    @@ -458,10 +458,10 @@ test_expect_success 'start preserves existing schedule' '
     '
     
     test_expect_success !MINGW 'start and stop macOS maintenance' '
    -	uid=$(id -u) &&
    +	uid=FAKE_UID &&
     
    -	write_script print-args <<-\EOF &&
    -	echo $* >>args
    +	write_script print-args <<-EOF &&
    +	echo \$* | perl -pe "s[(?<= gui/)-?[0-9]+][$uid]g" >>args
     	EOF
     
     	rm -f args &&

I.e. the context here is that the test is already hardcoding an assumption about "gui/%d" (per code in gc.c). It seems more robust & to the point of the test to not care about the specific UID number that comes back, since we're really testing whether we invoke our own code, not platform getuid() sanity.

Show 20 quoted lines

>  	write_script print-args <<-\EOF &&
>  	echo $* >>args
> @@ -432,7 +434,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '
>  	rm -f expect &&
>  	for frequency in hourly daily weekly
>  	do
> -		PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
> +		PLIST="$pfx/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
>  		test_xmllint "$PLIST" &&
>  		grep schedule=$frequency "$PLIST" &&
>  		echo "bootout gui/$uid $PLIST" >>expect &&
> @@ -446,7 +448,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '
>  	# stop does not unregister the repo
>  	git config --get --global maintenance.repo "$(pwd)" &&
>  
> -	printf "bootout gui/$uid $HOME/Library/LaunchAgents/org.git-scm.git.%s.plist\n" \
> +	printf "bootout gui/$uid $pfx/Library/LaunchAgents/org.git-scm.git.%s.plist\n" \
>  		hourly daily weekly >expect &&
>  	test_cmp expect args &&
>  	ls "$HOME/Library/LaunchAgents" >actual &&