{"thread":{"id":"54723","subject":"[PATCH 1/3] t7900: fix test failures when invoked individually via --run","startedAt":"2020-11-27T07:51:54Z","lastAt":"2020-11-28T22:16:57Z","messageCount":6,"participants":["Eric Sunshine","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"410917","messageId":"20201127075054.31174-2-sunshine@sunshineco.com","threadId":"54723","inReplyTo":"20201127075054.31174-1-sunshine@sunshineco.com","subject":"[PATCH 1/3] t7900: fix test failures when invoked individually via --run","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-11-27T07:50:52Z","receivedAt":"2020-11-27T07:51:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"A couple tests use `rm expect` to remove a file created by earlier\ntests. The presence of this file is immaterial at the point the attempt\nis made to remove it, yet if it doesn't exist, the test fails\nunnecessarily. One case in which the file may validly not exist is when\n--run or GIT_SKIP_TESTS is used to selectively run particular tests. Fix\nthese pointless failures.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t7900-maintenance.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 0246e4ce30..ef3aec3253 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -429,7 +429,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '\n \tEOF\n \ttest_cmp expect actual &&\n \n-\trm expect &&\n+\trm -f expect &&\n \tfor frequency in hourly daily weekly\n \tdo\n \t\tPLIST=\"$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist\" &&\n@@ -491,7 +491,6 @@ test_expect_success 'start and stop Windows maintenance' '\n \t# stop does not unregister the repo\n \tgit config --get --global maintenance.repo \"$(pwd)\" &&\n \n-\trm expect &&\n \tprintf \"/delete /tn Git Maintenance (%s) /f\\n\" \\\n \t\thourly daily weekly >expect &&\n \ttest_cmp expect args\n-- \n2.29.2.576.ga3fc446d84\n\n"},{"id":"410918","messageId":"20201127075054.31174-3-sunshine@sunshineco.com","threadId":"54723","inReplyTo":"20201127075054.31174-1-sunshine@sunshineco.com","subject":"[PATCH 2/3] test-tool: add `getuid` subcommand","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-11-27T07:50:53Z","receivedAt":"2020-11-27T07:51:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"It is difficult, if not impossible, for a test to obtain the user ID in\na portable fashion on all platforms and be guaranteed that it is the\nsame user ID Git itself sees when invoking getuid(). This is especially\ntrue on Microsoft Windows for which getuid() in C code and $(id -u) in a\nshell script may return entirely different values. Sidestep this problem\nby adding a `getuid` subcommand to test-tool which is guaranteed to\nprovide tests with the same value as getuid() returns in Git code.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n Makefile               | 1 +\n t/helper/test-getuid.c | 7 +++++++\n t/helper/test-tool.c   | 1 +\n t/helper/test-tool.h   | 1 +\n 4 files changed, 10 insertions(+)\n create mode 100644 t/helper/test-getuid.c\n\ndiff --git a/Makefile b/Makefile\nindex c39b39bd7d..a5c38aad5e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -703,6 +703,7 @@ TEST_BUILTINS_OBJS += test-dump-untracked-cache.o\n TEST_BUILTINS_OBJS += test-example-decorate.o\n TEST_BUILTINS_OBJS += test-genrandom.o\n TEST_BUILTINS_OBJS += test-genzeros.o\n+TEST_BUILTINS_OBJS += test-getuid.o\n TEST_BUILTINS_OBJS += test-hash-speed.o\n TEST_BUILTINS_OBJS += test-hash.o\n TEST_BUILTINS_OBJS += test-hashmap.o\ndiff --git a/t/helper/test-getuid.c b/t/helper/test-getuid.c\nnew file mode 100644\nindex 0000000000..d741302461\n--- /dev/null\n+++ b/t/helper/test-getuid.c\n@@ -0,0 +1,7 @@\n+#include \"test-tool.h\"\n+\n+int cmd__getuid(int argc, const char **argv)\n+{\n+\tprintf(\"%d\\n\", getuid());\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 432b49d948..ad5f681e68 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -31,6 +31,7 @@ static struct test_cmd cmds[] = {\n \t{ \"example-decorate\", cmd__example_decorate },\n \t{ \"genrandom\", cmd__genrandom },\n \t{ \"genzeros\", cmd__genzeros },\n+\t{ \"getuid\", cmd__getuid },\n \t{ \"hashmap\", cmd__hashmap },\n \t{ \"hash-speed\", cmd__hash_speed },\n \t{ \"index-version\", cmd__index_version },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 7c3281e071..765976e1eb 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -21,6 +21,7 @@ int cmd__dump_untracked_cache(int argc, const char **argv);\n int cmd__example_decorate(int argc, const char **argv);\n int cmd__genrandom(int argc, const char **argv);\n int cmd__genzeros(int argc, const char **argv);\n+int cmd__getuid(int argc, const char **argv);\n int cmd__hashmap(int argc, const char **argv);\n int cmd__hash_speed(int argc, const char **argv);\n int cmd__index_version(int argc, const char **argv);\n-- \n2.29.2.576.ga3fc446d84\n\n"},{"id":"410919","messageId":"20201127075054.31174-4-sunshine@sunshineco.com","threadId":"54723","inReplyTo":"20201127075054.31174-1-sunshine@sunshineco.com","subject":"[PATCH 3/3] t7900: make macOS-specific test work on Windows","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-11-27T07:50:54Z","receivedAt":"2020-11-27T07:51:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Although `git maintenance start` and `git maintenance stop` necessarily\ninvoke platform-specific scheduling utilities, their related tests have\nbeen carefully crafted -- with one minor exception -- to work correctly\non any platform, thus improving overall coverage. The exception is that\nthe macOS-specific test fails on Windows due to unportable use of\n`$(id -u)` and comparison involving the value of $HOME which suffers\nfrom the typical shortcoming on that platform in which the same path may\nbe represented two different ways depending upon its source (i.e. as a\nWindows path `C:/git-sdk-64/usr/src/git/foo` versus as a Unix path\n`/usr/src/git/foo`). Fix both problems and drop the !MINGW prerequisite\nfrom the macOS-specific test, thus allowing the test to run on Windows,\nas well.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t7900-maintenance.sh | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex ef3aec3253..500eaae4fd 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -408,8 +408,10 @@ test_expect_success 'start preserves existing schedule' '\n \tgrep \"Important information!\" cron.txt\n '\n \n-test_expect_success !MINGW 'start and stop macOS maintenance' '\n-\tuid=$(id -u) &&\n+test_expect_success 'start and stop macOS maintenance' '\n+\tuid=$(test-tool getuid) &&\n+\t# ensure $HOME can be compared against hook arguments on all platforms\n+\tpfx=$(cd \"$HOME\" && pwd) &&\n \n \twrite_script print-args <<-\\EOF &&\n \techo $* >>args\n@@ -432,7 +434,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '\n \trm -f expect &&\n \tfor frequency in hourly daily weekly\n \tdo\n-\t\tPLIST=\"$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist\" &&\n+\t\tPLIST=\"$pfx/Library/LaunchAgents/org.git-scm.git.$frequency.plist\" &&\n \t\ttest_xmllint \"$PLIST\" &&\n \t\tgrep schedule=$frequency \"$PLIST\" &&\n \t\techo \"bootout gui/$uid $PLIST\" >>expect &&\n@@ -446,7 +448,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '\n \t# stop does not unregister the repo\n \tgit config --get --global maintenance.repo \"$(pwd)\" &&\n \n-\tprintf \"bootout gui/$uid $HOME/Library/LaunchAgents/org.git-scm.git.%s.plist\\n\" \\\n+\tprintf \"bootout gui/$uid $pfx/Library/LaunchAgents/org.git-scm.git.%s.plist\\n\" \\\n \t\thourly daily weekly >expect &&\n \ttest_cmp expect args &&\n \tls \"$HOME/Library/LaunchAgents\" >actual &&\n-- \n2.29.2.576.ga3fc446d84\n\n"},{"id":"410920","messageId":"20201127075054.31174-1-sunshine@sunshineco.com","threadId":"54723","inReplyTo":null,"subject":"[PATCH 0/3] make macOS `git maintenance` test work on Windows","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-11-27T07:50:51Z","receivedAt":"2020-11-27T07:51:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The `git maintenance start` and `git maintenance stop` tests -- with one\nminor exception -- have been carefully crafted to work correctly on any\nplatform despite the fact that those commands necessarily invoke\nplatform-specific scheduling utilities. The exception is that the\nmacOS-specific test fails on Windows, thus is presently disabled on that\nplatform. This patch series addresses that shortcoming.\n\nThis series is built atop ds/maintenance-part-4.\n\nEric Sunshine (3):\n  t7900: fix test failures when invoked individually via --run\n  test-tool: add `getuid` subcommand\n  t7900: make macOS-specific test work on Windows\n\n Makefile               |  1 +\n t/helper/test-getuid.c |  7 +++++++\n t/helper/test-tool.c   |  1 +\n t/helper/test-tool.h   |  1 +\n t/t7900-maintenance.sh | 13 +++++++------\n 5 files changed, 17 insertions(+), 6 deletions(-)\n create mode 100644 t/helper/test-getuid.c\n\n-- \n2.29.2.576.ga3fc446d84\n\n"},{"id":"410933","messageId":"87o8jikfh7.fsf@evledraar.gmail.com","threadId":"54723","inReplyTo":"20201127075054.31174-4-sunshine@sunshineco.com","subject":"Re: [PATCH 3/3] t7900: make macOS-specific test work on Windows","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-27T15:05:08Z","receivedAt":"2020-11-27T15:05:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 27 2020, Eric Sunshine wrote:\n\n> Although `git maintenance start` and `git maintenance stop` necessarily\n> invoke platform-specific scheduling utilities, their related tests have\n> been carefully crafted -- with one minor exception -- to work correctly\n> on any platform, thus improving overall coverage. The exception is that\n> the macOS-specific test fails on Windows due to unportable use of\n> `$(id -u)` and comparison involving the value of $HOME which suffers\n> from the typical shortcoming on that platform in which the same path may\n> be represented two different ways depending upon its source (i.e. as a\n> Windows path `C:/git-sdk-64/usr/src/git/foo` versus as a Unix path\n> `/usr/src/git/foo`). Fix both problems and drop the !MINGW prerequisite\n> from the macOS-specific test, thus allowing the test to run on Windows,\n> as well.\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  t/t7900-maintenance.sh | 10 ++++++----\n>  1 file changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index ef3aec3253..500eaae4fd 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -408,8 +408,10 @@ test_expect_success 'start preserves existing schedule' '\n>  \tgrep \"Important information!\" cron.txt\n>  '\n>  \n> -test_expect_success !MINGW 'start and stop macOS maintenance' '\n> -\tuid=$(id -u) &&\n> +test_expect_success 'start and stop macOS maintenance' '\n> +\tuid=$(test-tool getuid) &&\n> +\t# ensure $HOME can be compared against hook arguments on all platforms\n> +\tpfx=$(cd \"$HOME\" && pwd) &&\n\nThis seems equally portable, and means your 2/3 isn't needed, no?\n    \n    diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n    index c3dcb9cb4d..b23f77aebc 100755\n    --- a/t/t7900-maintenance.sh\n    +++ b/t/t7900-maintenance.sh\n    @@ -458,10 +458,10 @@ test_expect_success 'start preserves existing schedule' '\n     '\n     \n     test_expect_success !MINGW 'start and stop macOS maintenance' '\n    -\tuid=$(id -u) &&\n    +\tuid=FAKE_UID &&\n     \n    -\twrite_script print-args <<-\\EOF &&\n    -\techo $* >>args\n    +\twrite_script print-args <<-EOF &&\n    +\techo \\$* | perl -pe \"s[(?<= gui/)-?[0-9]+][$uid]g\" >>args\n     \tEOF\n     \n     \trm -f args &&\n\nI.e. the context here is that the test is already hardcoding an\nassumption about \"gui/%d\" (per code in gc.c). It seems more robust & to\nthe point of the test to not care about the specific UID number that\ncomes back, since we're really testing whether we invoke our own code,\nnot platform getuid() sanity.\n\n\n>  \twrite_script print-args <<-\\EOF &&\n>  \techo $* >>args\n> @@ -432,7 +434,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '\n>  \trm -f expect &&\n>  \tfor frequency in hourly daily weekly\n>  \tdo\n> -\t\tPLIST=\"$HOME/Library/LaunchAgents/org.git-scm.git.$frequency.plist\" &&\n> +\t\tPLIST=\"$pfx/Library/LaunchAgents/org.git-scm.git.$frequency.plist\" &&\n>  \t\ttest_xmllint \"$PLIST\" &&\n>  \t\tgrep schedule=$frequency \"$PLIST\" &&\n>  \t\techo \"bootout gui/$uid $PLIST\" >>expect &&\n> @@ -446,7 +448,7 @@ test_expect_success !MINGW 'start and stop macOS maintenance' '\n>  \t# stop does not unregister the repo\n>  \tgit config --get --global maintenance.repo \"$(pwd)\" &&\n>  \n> -\tprintf \"bootout gui/$uid $HOME/Library/LaunchAgents/org.git-scm.git.%s.plist\\n\" \\\n> +\tprintf \"bootout gui/$uid $pfx/Library/LaunchAgents/org.git-scm.git.%s.plist\\n\" \\\n>  \t\thourly daily weekly >expect &&\n>  \ttest_cmp expect args &&\n>  \tls \"$HOME/Library/LaunchAgents\" >actual &&\n\n"},{"id":"410945","messageId":"CAPig+cRbga9ksi0JedU=U1zbLMY26xLtB7sBDJ5M5=TT1HsX1A@mail.gmail.com","threadId":"54723","inReplyTo":"87o8jikfh7.fsf@evledraar.gmail.com","subject":"Re: [PATCH 3/3] t7900: make macOS-specific test work on Windows","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-11-28T05:55:34Z","receivedAt":"2020-11-28T22:16:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 27, 2020 at 10:05 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Fri, Nov 27 2020, Eric Sunshine wrote:\n> > +test_expect_success 'start and stop macOS maintenance' '\n> > +     uid=$(test-tool getuid) &&\n>\n> This seems equally portable, and means your 2/3 isn't needed, no?\n>     +   uid=FAKE_UID &&\n>     +   echo \\$* | perl -pe \"s[(?<= gui/)-?[0-9]+][$uid]g\" >>args\n> I.e. the context here is that the test is already hardcoding an\n> assumption about \"gui/%d\" (per code in gc.c). It seems more robust & to\n> the point of the test to not care about the specific UID number that\n> comes back, since we're really testing whether we invoke our own code,\n> not platform getuid() sanity.\n\nGood idea. That's a reasonable viewpoint to take, and makes the series simpler.\n"}]}