* [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).