diff mbox series

[v2,2/2] t7900: make macOS-specific test work on Windows

Message ID 20201130044224.12298-3-sunshine@sunshineco.com (mailing list archive)
State New, archived
Headers show
Series make macOS `git maintenance` test work on Windows | expand

Commit Message

Eric Sunshine Nov. 30, 2020, 4:42 a.m. UTC
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 non-portable use of
`$(id -u)` and comparison involving the value of $HOME.

In particular, on Windows, the value of getuid() called by the C code is
not guaranteed to be the same as `$(id -u)` invoked by the test. This is
because `git.exe` is a native Windows program, whereas the utility
programs run by the test script mostly utilize the MSYS2 runtime, which
emulates a POSIX-like environment. Since the purpose of the test is to
check that the input to the hook is well-formed, the actual user ID is
immaterial, thus we can work around the problem by making the the test
UID-agnostic.

As for comparison of $HOME, it suffers from the typical shortcoming on
Windows 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.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7900-maintenance.sh | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Derrick Stolee Nov. 30, 2020, 1:12 p.m. UTC | #1
On 11/29/2020 11:42 PM, Eric Sunshine wrote:
> 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 non-portable use of
> `$(id -u)` and comparison involving the value of $HOME.

I appreciate your time making this change, as I was unable to work
around these issues myself.

> In particular, on Windows, the value of getuid() called by the C code is
> not guaranteed to be the same as `$(id -u)` invoked by the test. This is
> because `git.exe` is a native Windows program, whereas the utility
> programs run by the test script mostly utilize the MSYS2 runtime, which
> emulates a POSIX-like environment. Since the purpose of the test is to
> check that the input to the hook is well-formed, the actual user ID is
> immaterial, thus we can work around the problem by making the the test
> UID-agnostic.
> 
> As for comparison of $HOME, it suffers from the typical shortcoming on
> Windows 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.
> 
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t7900-maintenance.sh | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index ef3aec3253..514977a838 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -408,11 +408,12 @@ 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' '
> +	# ensure $HOME can be compared against hook arguments on all platforms
> +	pfx=$(cd "$HOME" && pwd) &&

> -		PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
> +		PLIST="$pfx/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&

This pair of changes make sense to get around the $HOME issue
that caused me to surrender the effort.

>  	write_script print-args <<-\EOF &&
> -	echo $* >>args
> +	echo $* | sed "s:gui/[0-9][0-9]*:gui/[UID]:" >>args

Filtering out the UID into a fixed string...

> -		echo "bootout gui/$uid $PLIST" >>expect &&
> -		echo "bootstrap gui/$uid $PLIST" >>expect || return 1
> +		echo "bootout gui/[UID] $PLIST" >>expect &&
> +		echo "bootstrap gui/[UID] $PLIST" >>expect || return 1
>  	done &&
>  	test_cmp expect args &&

.. then checking for that fixed string. Clever!

Thanks!
-Stolee
Eric Sunshine Dec. 1, 2020, 3:03 a.m. UTC | #2
On Mon, Nov 30, 2020 at 8:12 AM Derrick Stolee <stolee@gmail.com> wrote:
> On 11/29/2020 11:42 PM, Eric Sunshine wrote:
> > +     # ensure $HOME can be compared against hook arguments on all platforms
> > +     pfx=$(cd "$HOME" && pwd) &&
> > +             PLIST="$pfx/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
>
> This pair of changes make sense to get around the $HOME issue
> that caused me to surrender the effort.

In case it's not clear to other readers not familiar with Git on
Windows (or who have not read t/README), the magic here is that `pwd`
is overridden in t/test-lib.sh for MINGW to always return a
Windows-style path.
diff mbox series

Patch

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index ef3aec3253..514977a838 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -408,11 +408,12 @@  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' '
+	# ensure $HOME can be compared against hook arguments on all platforms
+	pfx=$(cd "$HOME" && pwd) &&
 
 	write_script print-args <<-\EOF &&
-	echo $* >>args
+	echo $* | sed "s:gui/[0-9][0-9]*:gui/[UID]:" >>args
 	EOF
 
 	rm -f args &&
@@ -432,11 +433,11 @@  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 &&
-		echo "bootstrap gui/$uid $PLIST" >>expect || return 1
+		echo "bootout gui/[UID] $PLIST" >>expect &&
+		echo "bootstrap gui/[UID] $PLIST" >>expect || return 1
 	done &&
 	test_cmp expect args &&
 
@@ -446,7 +447,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 &&