git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/3] make macOS `git maintenance` test work on Windows
@ 2020-11-27  7:50 Eric Sunshine
  2020-11-27  7:50 ` [PATCH 1/3] t7900: fix test failures when invoked individually via --run Eric Sunshine
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Eric Sunshine @ 2020-11-27  7:50 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Eric Sunshine

The `git maintenance start` and `git maintenance stop` tests -- with one
minor exception -- have been carefully crafted to work correctly on any
platform despite the fact that those commands necessarily invoke
platform-specific scheduling utilities. The exception is that the
macOS-specific test fails on Windows, thus is presently disabled on that
platform. This patch series addresses that shortcoming.

This series is built atop ds/maintenance-part-4.

Eric Sunshine (3):
  t7900: fix test failures when invoked individually via --run
  test-tool: add `getuid` subcommand
  t7900: make macOS-specific test work on Windows

 Makefile               |  1 +
 t/helper/test-getuid.c |  7 +++++++
 t/helper/test-tool.c   |  1 +
 t/helper/test-tool.h   |  1 +
 t/t7900-maintenance.sh | 13 +++++++------
 5 files changed, 17 insertions(+), 6 deletions(-)
 create mode 100644 t/helper/test-getuid.c

-- 
2.29.2.576.ga3fc446d84


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] t7900: fix test failures when invoked individually via --run
  2020-11-27  7:50 [PATCH 0/3] make macOS `git maintenance` test work on Windows Eric Sunshine
@ 2020-11-27  7:50 ` Eric Sunshine
  2020-11-27  7:50 ` [PATCH 2/3] test-tool: add `getuid` subcommand Eric Sunshine
  2020-11-27  7:50 ` [PATCH 3/3] t7900: make macOS-specific test work on Windows Eric Sunshine
  2 siblings, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2020-11-27  7:50 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Eric Sunshine

A couple tests use `rm expect` to remove a file created by earlier
tests. The presence of this file is immaterial at the point the attempt
is made to remove it, yet if it doesn't exist, the test fails
unnecessarily. One case in which the file may validly not exist is when
--run or GIT_SKIP_TESTS is used to selectively run particular tests. Fix
these pointless failures.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7900-maintenance.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 0246e4ce30..ef3aec3253 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -429,7 +429,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '
 	EOF
 	test_cmp expect actual &&
 
-	rm expect &&
+	rm -f expect &&
 	for frequency in hourly daily weekly
 	do
 		PLIST="$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist" &&
@@ -491,7 +491,6 @@ test_expect_success 'start and stop Windows maintenance' '
 	# stop does not unregister the repo
 	git config --get --global maintenance.repo "$(pwd)" &&
 
-	rm expect &&
 	printf "/delete /tn Git Maintenance (%s) /f\n" \
 		hourly daily weekly >expect &&
 	test_cmp expect args
-- 
2.29.2.576.ga3fc446d84


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] test-tool: add `getuid` subcommand
  2020-11-27  7:50 [PATCH 0/3] make macOS `git maintenance` test work on Windows Eric Sunshine
  2020-11-27  7:50 ` [PATCH 1/3] t7900: fix test failures when invoked individually via --run Eric Sunshine
@ 2020-11-27  7:50 ` Eric Sunshine
  2020-11-27  7:50 ` [PATCH 3/3] t7900: make macOS-specific test work on Windows Eric Sunshine
  2 siblings, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2020-11-27  7:50 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Eric Sunshine

It is difficult, if not impossible, for a test to obtain the user ID in
a portable fashion on all platforms and be guaranteed that it is the
same user ID Git itself sees when invoking getuid(). This is especially
true on Microsoft Windows for which getuid() in C code and $(id -u) in a
shell script may return entirely different values. Sidestep this problem
by adding a `getuid` subcommand to test-tool which is guaranteed to
provide tests with the same value as getuid() returns in Git code.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Makefile               | 1 +
 t/helper/test-getuid.c | 7 +++++++
 t/helper/test-tool.c   | 1 +
 t/helper/test-tool.h   | 1 +
 4 files changed, 10 insertions(+)
 create mode 100644 t/helper/test-getuid.c

diff --git a/Makefile b/Makefile
index c39b39bd7d..a5c38aad5e 100644
--- a/Makefile
+++ b/Makefile
@@ -703,6 +703,7 @@ TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
 TEST_BUILTINS_OBJS += test-example-decorate.o
 TEST_BUILTINS_OBJS += test-genrandom.o
 TEST_BUILTINS_OBJS += test-genzeros.o
+TEST_BUILTINS_OBJS += test-getuid.o
 TEST_BUILTINS_OBJS += test-hash-speed.o
 TEST_BUILTINS_OBJS += test-hash.o
 TEST_BUILTINS_OBJS += test-hashmap.o
diff --git a/t/helper/test-getuid.c b/t/helper/test-getuid.c
new file mode 100644
index 0000000000..d741302461
--- /dev/null
+++ b/t/helper/test-getuid.c
@@ -0,0 +1,7 @@
+#include "test-tool.h"
+
+int cmd__getuid(int argc, const char **argv)
+{
+	printf("%d\n", getuid());
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 432b49d948..ad5f681e68 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -31,6 +31,7 @@ static struct test_cmd cmds[] = {
 	{ "example-decorate", cmd__example_decorate },
 	{ "genrandom", cmd__genrandom },
 	{ "genzeros", cmd__genzeros },
+	{ "getuid", cmd__getuid },
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },
 	{ "index-version", cmd__index_version },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 7c3281e071..765976e1eb 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -21,6 +21,7 @@ int cmd__dump_untracked_cache(int argc, const char **argv);
 int cmd__example_decorate(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
 int cmd__genzeros(int argc, const char **argv);
+int cmd__getuid(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);
 int cmd__index_version(int argc, const char **argv);
-- 
2.29.2.576.ga3fc446d84


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] t7900: make macOS-specific test work on Windows
  2020-11-27  7:50 [PATCH 0/3] make macOS `git maintenance` test work on Windows Eric Sunshine
  2020-11-27  7:50 ` [PATCH 1/3] t7900: fix test failures when invoked individually via --run Eric Sunshine
  2020-11-27  7:50 ` [PATCH 2/3] test-tool: add `getuid` subcommand Eric Sunshine
@ 2020-11-27  7:50 ` Eric Sunshine
  2020-11-27 15:05   ` Ævar Arnfjörð Bjarmason
  2 siblings, 1 reply; 6+ messages in thread
From: Eric Sunshine @ 2020-11-27  7:50 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Eric Sunshine

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) &&
 
 	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 &&
-- 
2.29.2.576.ga3fc446d84


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] t7900: make macOS-specific test work on Windows
  2020-11-27  7:50 ` [PATCH 3/3] t7900: make macOS-specific test work on Windows Eric Sunshine
@ 2020-11-27 15:05   ` Ævar Arnfjörð Bjarmason
  2020-11-28  5:55     ` Eric Sunshine
  0 siblings, 1 reply; 6+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2020-11-27 15:05 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: git, Derrick Stolee


On Fri, Nov 27 2020, 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 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.


>  	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 &&


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] t7900: make macOS-specific test work on Windows
  2020-11-27 15:05   ` Ævar Arnfjörð Bjarmason
@ 2020-11-28  5:55     ` Eric Sunshine
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2020-11-28  5:55 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git List, Derrick Stolee

On Fri, Nov 27, 2020 at 10:05 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Fri, Nov 27 2020, Eric Sunshine wrote:
> > +test_expect_success 'start and stop macOS maintenance' '
> > +     uid=$(test-tool getuid) &&
>
> This seems equally portable, and means your 2/3 isn't needed, no?
>     +   uid=FAKE_UID &&
>     +   echo \$* | perl -pe "s[(?<= gui/)-?[0-9]+][$uid]g" >>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.

Good idea. That's a reasonable viewpoint to take, and makes the series simpler.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-28 22:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27  7:50 [PATCH 0/3] make macOS `git maintenance` test work on Windows Eric Sunshine
2020-11-27  7:50 ` [PATCH 1/3] t7900: fix test failures when invoked individually via --run Eric Sunshine
2020-11-27  7:50 ` [PATCH 2/3] test-tool: add `getuid` subcommand Eric Sunshine
2020-11-27  7:50 ` [PATCH 3/3] t7900: make macOS-specific test work on Windows Eric Sunshine
2020-11-27 15:05   ` Ævar Arnfjörð Bjarmason
2020-11-28  5:55     ` Eric Sunshine

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).