{"thread":{"id":"57297","subject":"[PATCH 0/9] ci: make Git's GitHub workflow output much more helpful","startedAt":"2022-01-24T19:01:38Z","lastAt":"2022-05-24T10:48:08Z","messageCount":98,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Johannes Schindelin","Ævar Arnfjörð Bjarmason","Junio C Hamano","Phillip Wood","Victoria Dye","Victoria Dye via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"446771","messageId":"pull.1117.git.1643050574.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":null,"subject":"[PATCH 0/9] ci: make Git's GitHub workflow output much more helpful","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:05Z","receivedAt":"2022-01-24T19:01:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nBackground\n==========\n\nRecent patches intended to help readers figure out CI failures much quicker\nthan before. Unfortunately, they haven't been entirely positive for me. For\nexample, they broke the branch protections in Microsoft's fork of Git, where\nwe require Pull Requests to pass a certain set of Checks (which are\nidentified by their names) and therefore caused follow-up work.\n\nUsing CI and in general making it easier for new contributors is an area I'm\npassionate about, and one I'd like to see improved.\n\n\nThe current situation\n=====================\n\nLet me walk you through the current experience when a PR build fails: I get\na notification mail that only says that a certain job failed. There's no\nindication of which test failed (or was it the build?). I can click on a\nlink at it takes me to the workflow run. Once there, all it says is \"Process\ncompleted with exit code 1\", or even \"code 2\". Sure, I can click on one of\nthe failed jobs. It even expands the failed step's log (collapsing the other\nsteps). And what do I see there?\n\nLet's look at an example of a failed linux-clang (ubuntu-latest) job\n[https://github.com/git-for-windows/git/runs/4822802185?check_suite_focus=true]:\n\n[...]\nTest Summary Report\n-------------------\nt1092-sparse-checkout-compatibility.sh (Wstat: 256 Tests: 53 Failed: 1)\n Failed test: 49\n Non-zero exit status: 1\nt3701-add-interactive.sh (Wstat: 0 Tests: 71 Failed: 0)\n TODO passed: 45, 47\nFiles=957, Tests=25489, 645 wallclock secs ( 5.74 usr 1.56 sys + 866.28 cusr 364.34 csys = 1237.92 CPU)\nResult: FAIL\nmake[1]: *** [Makefile:53: prove] Error 1\nmake[1]: Leaving directory '/home/runner/work/git/git/t'\nmake: *** [Makefile:3018: test] Error 2\n\n\nThat's it. I count myself lucky not to be a new contributor being faced with\nsomething like this.\n\nNow, since I am active in the Git project for a couple of days or so, I can\nmake sense of the \"TODO passed\" label and know that for the purpose of\nfixing the build failures, I need to ignore this, and that I need to focus\non the \"Failed test\" part instead.\n\nI also know that I do not have to get myself an ubuntu-latest box just to\nreproduce the error, I do not even have to check out the code and run it\njust to learn what that \"49\" means.\n\nI know, and I do not expect any new contributor, not even most seasoned\ncontributors to know, that I have to patiently collapse the \"Run\nci/run-build-and-tests.sh\" job's log, and instead expand the \"Run\nci/print-test-failures.sh\" job log (which did not fail and hence does not\ndraw any attention to it).\n\nI know, and again: I do not expect many others to know this, that I then\nhave to click into the \"Search logs\" box (not the regular web browser's\nsearch via Ctrl+F!) and type in \"not ok\" to find the log of the failed test\ncase (and this might still be a \"known broken\" one that is marked via\ntest_expect_failure and once again needs to be ignored).\n\nTo be excessively clear: This is not a great experience!\n\n\nImproved output\n===============\n\nOur previous Azure Pipelines-based CI builds had a much nicer UI, one that\neven showed flaky tests, and trends e.g. how long the test cases ran. When I\nported Git's CI over to GitHub workflows (to make CI more accessible to new\ncontributors), I knew fully well that we would leave this very nice UI\nbehind, and I had hoped that we would get something similar back via new,\ncommunity-contributed GitHub Actions that can be used in GitHub workflows.\nHowever, most likely because we use a home-grown test framework implemented\nin opinionated POSIX shells scripts, that did not happen.\n\nSo I had a look at what standards exist e.g. when testing PowerShell\nmodules, in the way of marking up their test output in GitHub workflows, and\nI was not disappointed: GitHub workflows support \"grouping\" of output lines,\ni.e. marking sections of the output as a group that is then collapsed by\ndefault and can be expanded. And it is this feature I decided to use in this\npatch series, along with GitHub workflows' commands to display errors or\nnotices that are also shown on the summary page of the workflow run. Now, in\naddition to \"Process completed with exit code\" on the summary page, we also\nread something like:\n\n⊗ linux-gcc (ubuntu-latest)\n failed: t9800.20 submit from detached head\n\n\nEven better, this message is a link, and following that, the reader is\npresented with something like this\n[https://github.com/dscho/git/runs/4840190622?check_suite_focus=true]:\n\n⏵ Run ci/run-build-and-tests.sh\n⏵ CI setup\n + ln -s /home/runner/none/.prove t/.prove\n + run_tests=t\n + export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n + group Build make\n + set +x\n⏵ Build\n⏵ Run tests\n === Failed test: t9800-git-p4-basic ===\n⏵ ok: t9800.1 start p4d\n⏵ ok: t9800.2 add p4 files\n⏵ ok: t9800.3 basic git p4 clone\n⏵ ok: t9800.4 depot typo error\n⏵ ok: t9800.5 git p4 clone @all\n⏵ ok: t9800.6 git p4 sync uninitialized repo\n⏵ ok: t9800.7 git p4 sync new branch\n⏵ ok: t9800.8 clone two dirs\n⏵ ok: t9800.9 clone two dirs, @all\n⏵ ok: t9800.10 clone two dirs, @all, conflicting files\n⏵ ok: t9800.11 clone two dirs, each edited by submit, single git commit\n⏵ ok: t9800.12 clone using non-numeric revision ranges\n⏵ ok: t9800.13 clone with date range, excluding some changes\n⏵ ok: t9800.14 exit when p4 fails to produce marshaled output\n⏵ ok: t9800.15 exit gracefully for p4 server errors\n⏵ ok: t9800.16 clone --bare should make a bare repository\n⏵ ok: t9800.17 initial import time from top change time\n⏵ ok: t9800.18 unresolvable host in P4PORT should display error\n⏵ ok: t9800.19 run hook p4-pre-submit before submit\n Error: failed: t9800.20 submit from detached head\n⏵ failure: t9800.20 submit from detached head \n Error: failed: t9800.21 submit from worktree\n⏵ failure: t9800.21 submit from worktree \n === Failed test: t9801-git-p4-branch ===\n [...]\n\n\nThe \"Failed test:\" lines are colored in yellow to give a better visual clue\nabout the logs' structure, the \"Error:\" label is colored in red to draw the\nattention to the important part of the log, and the \"⏵\" characters indicate\nthat part of the log is collapsed and can be expanded by clicking on it.\n\nTo drill down, the reader merely needs to expand the (failed) test case's\nlog by clicking on it, and then study the log. If needed (e.g. when the test\ncase relies on side effects from previous test cases), the logs of preceding\ntest cases can be expanded as well. In this example, when expanding\nt9800.20, it looks like this (for ease of reading, I cut a few chunks of\nlines, indicated by \"[...]\"):\n\n[...]\n⏵ ok: t9800.19 run hook p4-pre-submit before submit\n Error: failed: t9800.20 submit from detached head\n⏷ failure: t9800.20 submit from detached head \n test_when_finished cleanup_git &&\n git p4 clone --dest=\"$git\" //depot &&\n (\n cd \"$git\" &&\n git checkout p4/master &&\n >detached_head_test &&\n git add detached_head_test &&\n git commit -m \"add detached_head\" &&\n git config git-p4.skipSubmitEdit true &&\n git p4 submit &&\n git p4 rebase &&\n git log p4/master | grep detached_head\n )\n [...]\n Depot paths: //depot/\n Import destination: refs/remotes/p4/master\n \n Importing revision 9 (100%)Perforce db files in '.' will be created if missing...\n Perforce db files in '.' will be created if missing...\n \n Traceback (most recent call last):\n File \"/home/runner/work/git/git/git-p4\", line 4455, in \n main()\n File \"/home/runner/work/git/git/git-p4\", line 4449, in main\n if not cmd.run(args):\n File \"/home/runner/work/git/git/git-p4\", line 2590, in run\n rebase.rebase()\n File \"/home/runner/work/git/git/git-p4\", line 4121, in rebase\n if len(read_pipe(\"git diff-index HEAD --\")) > 0:\n File \"/home/runner/work/git/git/git-p4\", line 297, in read_pipe\n retcode, out, err = read_pipe_full(c, *k, **kw)\n File \"/home/runner/work/git/git/git-p4\", line 284, in read_pipe_full\n p = subprocess.Popen(\n File \"/usr/lib/python3.8/subprocess.py\", line 858, in __init__\n self._execute_child(args, executable, preexec_fn, close_fds,\n File \"/usr/lib/python3.8/subprocess.py\", line 1704, in _execute_child\n raise child_exception_type(errno_num, err_msg, err_filename)\n FileNotFoundError: [Errno 2] No such file or directory: 'git diff-index HEAD --'\n error: last command exited with $?=1\n + cleanup_git\n + retry_until_success rm -r /home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git\n + nr_tries_left=60\n + rm -r /home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git\n + test_path_is_missing /home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git\n + test 1 -ne 1\n + test -e /home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git\n + retry_until_success mkdir /home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git\n + nr_tries_left=60\n + mkdir /home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git\n + exit 1\n + eval_ret=1\n + :\n not ok 20 - submit from detached head\n # \n # test_when_finished cleanup_git &&\n # git p4 clone --dest=\"$git\" //depot &&\n # (\n # cd \"$git\" &&\n # git checkout p4/master &&\n # >detached_head_test &&\n # git add detached_head_test &&\n # git commit -m \"add detached_head\" &&\n # git config git-p4.skipSubmitEdit true &&\n # git p4 submit &&\n # git p4 rebase &&\n # git log p4/master | grep detached_head\n # )\n # \n Error: failed: t9800.21 submit from worktree\n [...]\n\n\nIs this the best UI we can have for test failures in CI runs? I hope we can\ndo better. Having said that, this patch series presents a pretty good start,\nand offers a basis for future improvements.\n\nJohannes Schindelin (9):\n ci: fix code style\n ci/run-build-and-tests: take a more high-level view\n ci: make it easier to find failed tests' logs in the GitHub workflow\n ci/run-build-and-tests: add some structure to the GitHub workflow\n output\n tests: refactor --write-junit-xml code\n test(junit): avoid line feeds in XML attributes\n ci: optionally mark up output in the GitHub workflow\n ci: use `--github-workflow-markup` in the GitHub workflow\n ci: call `finalize_test_case_output` a little later\n\n .github/workflows/main.yml | 12 ---\n ci/lib.sh | 81 ++++++++++++++--\n ci/run-build-and-tests.sh | 11 ++-\n ci/run-test-slice.sh | 5 +-\n t/test-lib-functions.sh | 4 +-\n t/test-lib-github-workflow-markup.sh | 50 ++++++++++\n t/test-lib-junit.sh | 132 +++++++++++++++++++++++++++\n t/test-lib.sh | 128 ++++----------------------\n 8 files changed, 287 insertions(+), 136 deletions(-)\n create mode 100644 t/test-lib-github-workflow-markup.sh\n create mode 100644 t/test-lib-junit.sh\n\n\nbase-commit: af4e5f569bc89f356eb34a9373d7f82aca6faa8a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1117%2Fdscho%2Fuse-grouping-in-ci-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1117/dscho/use-grouping-in-ci-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1117\n-- \ngitgitgadget\n"},{"id":"446772","messageId":"db08b07c37a81866c63b062c92d9ee78976d1b34.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 1/9] ci: fix code style","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:06Z","receivedAt":"2022-01-24T19:01:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn b92cb86ea14 (travis-ci: check that all build artifacts are\n.gitignore-d, 2017-12-31), a function was introduced with a code style\nthat is different from the surrounding code: it added the opening curly\nbrace on its own line, when all the existing functions in the same file\ncuddle that brace on the same line as the function name.\n\nLet's make the code style consistent again.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/lib.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 9d28ab50fb4..ebb502640fa 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -69,8 +69,7 @@ skip_good_tree () {\n \texit 0\n }\n \n-check_unignored_build_artifacts ()\n-{\n+check_unignored_build_artifacts () {\n \t! git ls-files --other --exclude-standard --error-unmatch \\\n \t\t-- ':/*' 2>/dev/null ||\n \t{\n-- \ngitgitgadget\n\n"},{"id":"446773","messageId":"d2ff51bb5dacfe084166de106e9a864c902f0f36.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 2/9] ci/run-build-and-tests: take a more high-level view","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:07Z","receivedAt":"2022-01-24T19:01:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the web UI of GitHub workflows, failed runs are presented with the\njob step that failed auto-expanded. In the current setup, this is not\nhelpful at all because that shows only the output of `prove`, which says\nwhich test failed, but not in what way.\n\nWhat would help understand the reader what went wrong is the verbose\ntest output of the failed test.\n\nThe logs of the failed runs do contain that verbose test output, but it\nis shown in the _next_ step (which is marked as succeeding, and is\ntherefore _not_ auto-expanded). Anyone not intimately familiar with this\nwould completely miss the verbose test output, being left mostly\npuzzled with the test failures.\n\nWe are about to show the failed test cases' output in the _same_ step,\nso that the user has a much easier time to figure out what was going\nwrong.\n\nBut first, we must partially revert the change that tried to improve the\nCI runs by combining the `Makefile` targets to build into a single\n`make` invocation. That might have sounded like a good idea at the time,\nbut it does make it rather impossible for the CI script to determine\nwhether the _build_ failed, or the _tests_. If the tests were run at\nall, that is.\n\nSo let's go back to calling `make` for the build, and call `make test`\nseparately so that we can easily detect that _that_ invocation failed,\nand react appropriately.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/run-build-and-tests.sh | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 280dda7d285..b70373c172f 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -10,7 +10,7 @@ windows*) cmd //c mklink //j t\\\\.prove \"$(cygpath -aw \"$cache_dir/.prove\")\";;\n *) ln -s \"$cache_dir/.prove\" t/.prove;;\n esac\n \n-export MAKE_TARGETS=\"all test\"\n+run_tests=t\n \n case \"$jobname\" in\n linux-gcc)\n@@ -41,14 +41,18 @@ pedantic)\n \t# Don't run the tests; we only care about whether Git can be\n \t# built.\n \texport DEVOPTS=pedantic\n-\texport MAKE_TARGETS=all\n+\trun_tests=\n \t;;\n esac\n \n # Any new \"test\" targets should not go after this \"make\", but should\n # adjust $MAKE_TARGETS. Otherwise compilation-only targets above will\n # start running tests.\n-make $MAKE_TARGETS\n+make\n+if test -n \"$run_tests\"\n+then\n+\tmake test\n+fi\n check_unignored_build_artifacts\n \n save_good_tree\n-- \ngitgitgadget\n\n"},{"id":"446774","messageId":"98891b0d3f7927086350cc9523736ae2eb7c1ee2.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 3/9] ci: make it easier to find failed tests' logs in the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:08Z","receivedAt":"2022-01-24T19:01:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nYou currently have to know a lot of implementation details when\ninvestigating test failures in the CI runs. The first step is easy: the\nfailed job is marked quite clearly, but when opening it, the failed step\nis expanded, which in our case is the one running\n`ci/run-build-and-tests.sh`. This step, most notably, only offers a\nhigh-level view of what went wrong: it prints the output of `prove`\nwhich merely tells the reader which test script failed.\n\nThe actually interesting part is in the detailed log of said failed\ntest script. But that log is shown in the CI run's step that runs\n`ci/print-test-failures.sh`. And that step is _not_ expanded in the web\nUI by default.\n\nLet's help the reader by showing the failed tests' detailed logs in the\nstep that is expanded automatically, i.e. directly after the test suite\nfailed.\n\nThis also helps the situation where the _build_ failed and the\n`print-test-failures` step was executed under the assumption that the\n_test suite_ failed, and consequently failed to find any failed tests.\n\nAn alternative way to implement this patch would be to source\n`ci/print-test-failures.sh` in the `handle_test_failures` function to\nshow these logs. However, over the course of the next few commits, we\nwant to introduce some grouping which would be harder to achieve that\nway (for example, we do want a leaner, and colored, preamble for each\nfailed test script, and it would be trickier to accommodate the lack of\nnested groupings in GitHub workflows' output).\n\nSigned-off-by: Johannes Schindelin \n---\n .github/workflows/main.yml | 12 ------------\n ci/lib.sh | 23 +++++++++++++++++++++++\n ci/run-build-and-tests.sh | 3 ++-\n ci/run-test-slice.sh | 3 ++-\n 4 files changed, 27 insertions(+), 14 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex c35200defb9..3fa88b78b6d 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -119,10 +119,6 @@ jobs:\n - name: test\n shell: bash\n run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n- - name: ci/print-test-failures.sh\n- if: failure()\n- shell: bash\n- run: ci/print-test-failures.sh\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v2\n@@ -204,10 +200,6 @@ jobs:\n env:\n NO_SVN_TESTS: 1\n run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n- - name: ci/print-test-failures.sh\n- if: failure()\n- shell: bash\n- run: ci/print-test-failures.sh\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v2\n@@ -261,8 +253,6 @@ jobs:\n - uses: actions/checkout@v2\n - run: ci/install-dependencies.sh\n - run: ci/run-build-and-tests.sh\n- - run: ci/print-test-failures.sh\n- if: failure()\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v2\n@@ -292,8 +282,6 @@ jobs:\n - uses: actions/checkout@v1\n - run: ci/install-docker-dependencies.sh\n - run: ci/run-build-and-tests.sh\n- - run: ci/print-test-failures.sh\n- if: failure()\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v1\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex ebb502640fa..2b2c0932320 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -78,6 +78,10 @@ check_unignored_build_artifacts () {\n \t}\n }\n \n+handle_failed_tests () {\n+\treturn 1\n+}\n+\n # GitHub Action doesn't set TERM, which is required by tput\n export TERM=${TERM:-dumb}\n \n@@ -123,6 +127,25 @@ then\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n \tCC=\"${CC:-gcc}\"\n \tDONT_SKIP_TAGS=t\n+\thandle_failed_tests () {\n+\t\tmkdir -p t/failed-test-artifacts\n+\t\techo \"FAILED_TEST_ARTIFACTS=t/failed-test-artifacts\" >>$GITHUB_ENV\n+\n+\t\tfor test_exit in t/test-results/*.exit\n+\t\tdo\n+\t\t\ttest 0 != \"$(cat \"$test_exit\")\" || continue\n+\n+\t\t\ttest_name=\"${test_exit%.exit}\"\n+\t\t\ttest_name=\"${test_name##*/}\"\n+\t\t\tprintf \"\\\\e[33m\\\\e[1m=== Failed test: ${test_name} ===\\\\e[m\\\\n\"\n+\t\t\tcat \"t/test-results/$test_name.out\"\n+\n+\t\t\ttrash_dir=\"t/trash directory.$test_name\"\n+\t\t\tcp \"t/test-results/$test_name.out\" t/failed-test-artifacts/\n+\t\t\ttar czf t/failed-test-artifacts/\"$test_name\".trash.tar.gz \"$trash_dir\"\n+\t\tdone\n+\t\treturn 1\n+\t}\n \n \tcache_dir=\"$HOME/none\"\n \ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex b70373c172f..e49f9eaa8c0 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -51,7 +51,8 @@ esac\n make\n if test -n \"$run_tests\"\n then\n-\tmake test\n+\tmake test ||\n+\thandle_failed_tests\n fi\n check_unignored_build_artifacts\n \ndiff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh\nindex f8c2c3106a2..63358c23e11 100755\n--- a/ci/run-test-slice.sh\n+++ b/ci/run-test-slice.sh\n@@ -12,6 +12,7 @@ esac\n \n make --quiet -C t T=\"$(cd t &&\n \t./helper/test-tool path-utils slice-tests \"$1\" \"$2\" t[0-9]*.sh |\n-\ttr '\\n' ' ')\"\n+\ttr '\\n' ' ')\" ||\n+handle_failed_tests\n \n check_unignored_build_artifacts\n-- \ngitgitgadget\n\n"},{"id":"446775","messageId":"9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 4/9] ci/run-build-and-tests: add some structure to the GitHub workflow output","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:09Z","receivedAt":"2022-01-24T19:01:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe current output of Git's GitHub workflow can be quite confusing,\nespecially for contributors new to the project.\n\nTo make it more helpful, let's introduce some collapsible grouping.\nInitially, readers will see the high-level view of what actually\nhappened (did the build fail, or the test suite?). To drill down, the\nrespective group can be expanded.\n\nNote: sadly, workflow output currently cannot contain any nested groups\n(see https://github.com/actions/runner/issues/802 for details),\ntherefore we take pains to ensure to end any previous group before\nstarting a new one.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/lib.sh | 55 ++++++++++++++++++++++++++++++++++-----\n ci/run-build-and-tests.sh | 4 +--\n ci/run-test-slice.sh | 2 +-\n 3 files changed, 51 insertions(+), 10 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 2b2c0932320..4ed8f40ab02 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -1,5 +1,49 @@\n # Library of functions shared by all CI scripts\n \n+if test true != \"$GITHUB_ACTIONS\"\n+then\n+\tbegin_group () { :; }\n+\tend_group () { :; }\n+\n+\tgroup () {\n+\t\tshift\n+\t\t\"$@\"\n+\t}\n+\tset -x\n+else\n+\tbegin_group () {\n+\t\tneed_to_end_group=t\n+\t\techo \"::group::$1\" >&2\n+\t\tset -x\n+\t}\n+\n+\tend_group () {\n+\t\ttest -n \"$need_to_end_group\" || return 0\n+\t\tset +x\n+\t\tneed_to_end_group=\n+\t\techo '::endgroup::' >&2\n+\t}\n+\ttrap end_group EXIT\n+\n+\tgroup () {\n+\t\tset +x\n+\t\tbegin_group \"$1\"\n+\t\tshift\n+\t\t\"$@\"\n+\t\tres=$?\n+\t\tend_group\n+\t\treturn $res\n+\t}\n+\n+\tbegin_group \"CI setup\"\n+fi\n+\n+# Set 'exit on error' for all CI scripts to let the caller know that\n+# something went wrong.\n+# Set tracing executed commands, primarily setting environment variables\n+# and installing dependencies.\n+set -e\n+\n skip_branch_tip_with_tag () {\n \t# Sometimes, a branch is pushed at the same time the tag that points\n \t# at the same commit as the tip of the branch is pushed, and building\n@@ -88,12 +132,6 @@ export TERM=${TERM:-dumb}\n # Clear MAKEFLAGS that may come from the outside world.\n export MAKEFLAGS=\n \n-# Set 'exit on error' for all CI scripts to let the caller know that\n-# something went wrong.\n-# Set tracing executed commands, primarily setting environment variables\n-# and installing dependencies.\n-set -ex\n-\n if test -n \"$SYSTEM_COLLECTIONURI\" || test -n \"$SYSTEM_TASKDEFINITIONSURI\"\n then\n \tCI_TYPE=azure-pipelines\n@@ -138,7 +176,7 @@ then\n \t\t\ttest_name=\"${test_exit%.exit}\"\n \t\t\ttest_name=\"${test_name##*/}\"\n \t\t\tprintf \"\\\\e[33m\\\\e[1m=== Failed test: ${test_name} ===\\\\e[m\\\\n\"\n-\t\t\tcat \"t/test-results/$test_name.out\"\n+\t\t\tgroup \"Failed test: $test_name\" cat \"t/test-results/$test_name.out\"\n \n \t\t\ttrash_dir=\"t/trash directory.$test_name\"\n \t\t\tcp \"t/test-results/$test_name.out\" t/failed-test-artifacts/\n@@ -234,3 +272,6 @@ linux-leaks)\n esac\n \n MAKEFLAGS=\"$MAKEFLAGS CC=${CC:-cc}\"\n+\n+end_group\n+set -x\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex e49f9eaa8c0..5516f45f7fe 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -48,10 +48,10 @@ esac\n # Any new \"test\" targets should not go after this \"make\", but should\n # adjust $MAKE_TARGETS. Otherwise compilation-only targets above will\n # start running tests.\n-make\n+group Build make\n if test -n \"$run_tests\"\n then\n-\tmake test ||\n+\tgroup \"Run tests\" make test ||\n \thandle_failed_tests\n fi\n check_unignored_build_artifacts\ndiff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh\nindex 63358c23e11..a3c67956a8d 100755\n--- a/ci/run-test-slice.sh\n+++ b/ci/run-test-slice.sh\n@@ -10,7 +10,7 @@ windows*) cmd //c mklink //j t\\\\.prove \"$(cygpath -aw \"$cache_dir/.prove\")\";;\n *) ln -s \"$cache_dir/.prove\" t/.prove;;\n esac\n \n-make --quiet -C t T=\"$(cd t &&\n+group \"Run tests\" make --quiet -C t T=\"$(cd t &&\n \t./helper/test-tool path-utils slice-tests \"$1\" \"$2\" t[0-9]*.sh |\n \ttr '\\n' ' ')\" ||\n handle_failed_tests\n-- \ngitgitgadget\n\n"},{"id":"446776","messageId":"94dcbe1bc437dc9738cfc635e9085d743eb4ec98.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 5/9] tests: refactor --write-junit-xml code","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:10Z","receivedAt":"2022-01-24T19:01:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe code writing JUnit XML is interspersed directly with all the code in\n`t/test-lib.sh`, and it is therefore not only ill-separated, but\nintroducing yet another output format would make the situation even\nworse.\n\nLet's introduce an abstraction layer by hiding the JUnit XML code behind\nfour new functions that are supposed to be called before and after each\ntest and test case.\n\nThis is not just an academic exercise, refactoring for refactoring's\nsake. We _actually_ want to introduce such a new output format, to\nmake it substantially easier to diagnose test failures in our GitHub\nworkflow, therefore we do need this refactoring.\n\nThis commit is best viewed with `git show --color-moved\n--color-moved-ws=allow-indentation-change `.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib-junit.sh | 126 ++++++++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh | 124 ++++++-------------------------------------\n 2 files changed, 142 insertions(+), 108 deletions(-)\n create mode 100644 t/test-lib-junit.sh\n\ndiff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh\nnew file mode 100644\nindex 00000000000..9d55d74d764\n--- /dev/null\n+++ b/t/test-lib-junit.sh\n@@ -0,0 +1,126 @@\n+# Library of functions to format test scripts' output in JUnit XML\n+# format, to support Git's test suite result to be presented in an\n+# easily digestible way on Azure Pipelines.\n+#\n+# Copyright (c) 2022 Johannes Schindelin\n+#\n+# This program is free software: you can redistribute it and/or modify\n+# it under the terms of the GNU General Public License as published by\n+# the Free Software Foundation, either version 2 of the License, or\n+# (at your option) any later version.\n+#\n+# This program is distributed in the hope that it will be useful,\n+# but WITHOUT ANY WARRANTY; without even the implied warranty of\n+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n+# GNU General Public License for more details.\n+#\n+# You should have received a copy of the GNU General Public License\n+# along with this program. If not, see http://www.gnu.org/licenses/ .\n+#\n+# The idea is for `test-lib.sh` to source this file when the user asks\n+# for JUnit XML; these functions will then override (empty) functions\n+# that are are called at the appropriate times during the test runs.\n+\n+start_test_output () {\n+\tjunit_xml_dir=\"$TEST_OUTPUT_DIRECTORY/out\"\n+\tmkdir -p \"$junit_xml_dir\"\n+\tjunit_xml_base=${1##*/}\n+\tjunit_xml_path=\"$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml\"\n+\tjunit_attrs=\"name=\\\"${junit_xml_base%.sh}\\\"\"\n+\tjunit_attrs=\"$junit_attrs timestamp=\\\"$(TZ=UTC \\\n+\t\tdate +%Y-%m-%dT%H:%M:%S)\\\"\"\n+\twrite_junit_xml --truncate \"\" \" \"\n+\tjunit_suite_start=$(test-tool date getnanos)\n+\tif test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n+\tthen\n+\t\tGIT_TEST_TEE_OFFSET=0\n+\tfi\n+}\n+\n+start_test_case_output () {\n+\tjunit_start=$(test-tool date getnanos)\n+}\n+\n+finalize_test_case_output () {\n+\ttest_case_result=$1\n+\tshift\n+\tcase \"$test_case_result\" in\n+\tok)\n+\t\tset \"$*\"\n+\t\t;;\n+\tfailure)\n+\t\tjunit_insert=\"\"\n+\t\tjunit_insert=\"$junit_insert $(xml_attr_encode \\\n+\t\t\t\"$(if test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n+\t\t\t then\n+\t\t\t\ttest-tool path-utils skip-n-bytes \\\n+\t\t\t\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n+\t\t\t else\n+\t\t\t\tprintf '%s\\n' \"$@\" | sed 1d\n+\t\t\t fi)\")\"\n+\t\tjunit_insert=\"$junit_insert\"\n+\t\tif test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n+\t\tthen\n+\t\t\tjunit_insert=\"$junit_insert$(xml_attr_encode \\\n+\t\t\t\t\"$(cat \"$GIT_TEST_TEE_OUTPUT_FILE\")\")\"\n+\t\tfi\n+\t\tset \"$1\" \" $junit_insert\"\n+\t\t;;\n+\tfixed)\n+\t\tset \"$* (breakage fixed)\"\n+\t\t;;\n+\tbroken)\n+\t\tset \"$* (known breakage)\"\n+\t\t;;\n+\tskip)\n+\t\tmessage=\"$(xml_attr_encode \"$skipped_reason\")\"\n+\t\tset \"$1\" \" \"\n+\t\t;;\n+\tesac\n+\n+\tjunit_attrs=\"name=\\\"$(xml_attr_encode \"$this_test.$test_count $1\")\\\"\"\n+\tshift\n+\tjunit_attrs=\"$junit_attrs classname=\\\"$this_test\\\"\"\n+\tjunit_attrs=\"$junit_attrs time=\\\"$(test-tool \\\n+\t\tdate getnanos $junit_start)\\\"\"\n+\twrite_junit_xml \"$(printf '%s\\n' \\\n+\t\t\" \" \"$@\" \" \")\"\n+\tjunit_have_testcase=t\n+}\n+\n+finalize_test_output () {\n+\tif test -n \"$junit_xml_path\"\n+\tthen\n+\t\ttest -n \"$junit_have_testcase\" || {\n+\t\t\tjunit_start=$(test-tool date getnanos)\n+\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n+\t\t}\n+\n+\t\t# adjust the overall time\n+\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n+\t\tsed -e \"s/\\(]*/& time=\\\"$junit_time\\\"/\" \\\n+\t\t\t-e '/^ *<\\/testsuite/d' \\\n+\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n+\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n+\n+\t\twrite_junit_xml \" \" \"\"\n+\t\twrite_junit_xml=\n+\tfi\n+}\n+\n+write_junit_xml () {\n+\tcase \"$1\" in\n+\t--truncate)\n+\t\t>\"$junit_xml_path\"\n+\t\tjunit_have_testcase=\n+\t\tshift\n+\t\t;;\n+\tesac\n+\tprintf '%s\\n' \"$@\" >>\"$junit_xml_path\"\n+}\n+\n+xml_attr_encode () {\n+\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+}\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0f7a137c7d8..e13e1cb9124 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -107,6 +107,12 @@ mark_option_requires_arg () {\n \tstore_arg_to=$2\n }\n \n+# These functions can be overridden e.g. to output JUnit XML\n+start_test_output () { :; }\n+start_test_case_output () { :; }\n+finalize_test_case_output () { :; }\n+finalize_test_output () { :; }\n+\n parse_option () {\n \tlocal opt=\"$1\"\n \n@@ -166,7 +172,7 @@ parse_option () {\n \t\ttee=t\n \t\t;;\n \t--write-junit-xml)\n-\t\twrite_junit_xml=t\n+\t\t. \"$TEST_DIRECTORY/test-lib-junit.sh\"\n \t\t;;\n \t--stress)\n \t\tstress=t ;;\n@@ -613,7 +619,7 @@ exec 6<&0\n exec 7>&2\n \n _error_exit () {\n-\tfinalize_junit_xml\n+\tfinalize_test_output\n \tGIT_EXIT_OK=t\n \texit 1\n }\n@@ -723,35 +729,13 @@ trap '{ code=$?; set +x; } 2>/dev/null; exit $code' INT TERM HUP\n # the test_expect_* functions instead.\n \n test_ok_ () {\n-\tif test -n \"$write_junit_xml\"\n-\tthen\n-\t\twrite_junit_xml_testcase \"$*\"\n-\tfi\n+\tfinalize_test_case_output ok \"$@\"\n \ttest_success=$(($test_success + 1))\n \tsay_color \"\" \"ok $test_count - $@\"\n }\n \n test_failure_ () {\n-\tif test -n \"$write_junit_xml\"\n-\tthen\n-\t\tjunit_insert=\"\"\n-\t\tjunit_insert=\"$junit_insert $(xml_attr_encode \\\n-\t\t\t\"$(if test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n-\t\t\t then\n-\t\t\t\ttest-tool path-utils skip-n-bytes \\\n-\t\t\t\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n-\t\t\t else\n-\t\t\t\tprintf '%s\\n' \"$@\" | sed 1d\n-\t\t\t fi)\")\"\n-\t\tjunit_insert=\"$junit_insert\"\n-\t\tif test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n-\t\tthen\n-\t\t\tjunit_insert=\"$junit_insert$(xml_attr_encode \\\n-\t\t\t\t\"$(cat \"$GIT_TEST_TEE_OUTPUT_FILE\")\")\"\n-\t\tfi\n-\t\twrite_junit_xml_testcase \"$1\" \" $junit_insert\"\n-\tfi\n+\tfinalize_test_case_output failure \"$@\"\n \ttest_failure=$(($test_failure + 1))\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n@@ -760,19 +744,13 @@ test_failure_ () {\n }\n \n test_known_broken_ok_ () {\n-\tif test -n \"$write_junit_xml\"\n-\tthen\n-\t\twrite_junit_xml_testcase \"$* (breakage fixed)\"\n-\tfi\n+\tfinalize_test_case_output fixed \"$@\"\n \ttest_fixed=$(($test_fixed+1))\n \tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n }\n \n test_known_broken_failure_ () {\n-\tif test -n \"$write_junit_xml\"\n-\tthen\n-\t\twrite_junit_xml_testcase \"$* (known breakage)\"\n-\tfi\n+\tfinalize_test_case_output broken \"$@\"\n \ttest_broken=$(($test_broken+1))\n \tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n }\n@@ -1049,10 +1027,7 @@ test_start_ () {\n \ttest_count=$(($test_count+1))\n \tmaybe_setup_verbose\n \tmaybe_setup_valgrind\n-\tif test -n \"$write_junit_xml\"\n-\tthen\n-\t\tjunit_start=$(test-tool date getnanos)\n-\tfi\n+\tstart_test_case_output\n }\n \n test_finish_ () {\n@@ -1103,12 +1078,7 @@ test_skip () {\n \n \tcase \"$to_skip\" in\n \tt)\n-\t\tif test -n \"$write_junit_xml\"\n-\t\tthen\n-\t\t\tmessage=\"$(xml_attr_encode \"$skipped_reason\")\"\n-\t\t\twrite_junit_xml_testcase \"$1\" \\\n-\t\t\t\t\" \"\n-\t\tfi\n+\t\tfinalize_test_case_output skip \"$@\"\n \n \t\tsay_color skip \"ok $test_count # skip $1 ($skipped_reason)\"\n \t\t: true\n@@ -1124,53 +1094,6 @@ test_at_end_hook_ () {\n \t:\n }\n \n-write_junit_xml () {\n-\tcase \"$1\" in\n-\t--truncate)\n-\t\t>\"$junit_xml_path\"\n-\t\tjunit_have_testcase=\n-\t\tshift\n-\t\t;;\n-\tesac\n-\tprintf '%s\\n' \"$@\" >>\"$junit_xml_path\"\n-}\n-\n-xml_attr_encode () {\n-\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n-}\n-\n-write_junit_xml_testcase () {\n-\tjunit_attrs=\"name=\\\"$(xml_attr_encode \"$this_test.$test_count $1\")\\\"\"\n-\tshift\n-\tjunit_attrs=\"$junit_attrs classname=\\\"$this_test\\\"\"\n-\tjunit_attrs=\"$junit_attrs time=\\\"$(test-tool \\\n-\t\tdate getnanos $junit_start)\\\"\"\n-\twrite_junit_xml \"$(printf '%s\\n' \\\n-\t\t\" \" \"$@\" \" \")\"\n-\tjunit_have_testcase=t\n-}\n-\n-finalize_junit_xml () {\n-\tif test -n \"$write_junit_xml\" && test -n \"$junit_xml_path\"\n-\tthen\n-\t\ttest -n \"$junit_have_testcase\" || {\n-\t\t\tjunit_start=$(test-tool date getnanos)\n-\t\t\twrite_junit_xml_testcase \"all tests skipped\"\n-\t\t}\n-\n-\t\t# adjust the overall time\n-\t\tjunit_time=$(test-tool date getnanos $junit_suite_start)\n-\t\tsed -e \"s/\\(]*/& time=\\\"$junit_time\\\"/\" \\\n-\t\t\t-e '/^ *<\\/testsuite/d' \\\n-\t\t\t<\"$junit_xml_path\" >\"$junit_xml_path.new\"\n-\t\tmv \"$junit_xml_path.new\" \"$junit_xml_path\"\n-\n-\t\twrite_junit_xml \" \" \"\"\n-\t\twrite_junit_xml=\n-\tfi\n-}\n-\n test_atexit_cleanup=:\n test_atexit_handler () {\n \t# In a succeeding test script 'test_atexit_handler' is invoked\n@@ -1193,7 +1116,7 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tfinalize_junit_xml\n+\tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n \tthen\n@@ -1484,22 +1407,7 @@ fi\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n-if test -n \"$write_junit_xml\"\n-then\n-\tjunit_xml_dir=\"$TEST_OUTPUT_DIRECTORY/out\"\n-\tmkdir -p \"$junit_xml_dir\"\n-\tjunit_xml_base=${0##*/}\n-\tjunit_xml_path=\"$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml\"\n-\tjunit_attrs=\"name=\\\"${junit_xml_base%.sh}\\\"\"\n-\tjunit_attrs=\"$junit_attrs timestamp=\\\"$(TZ=UTC \\\n-\t\tdate +%Y-%m-%dT%H:%M:%S)\\\"\"\n-\twrite_junit_xml --truncate \"\" \" \"\n-\tjunit_suite_start=$(test-tool date getnanos)\n-\tif test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n-\tthen\n-\t\tGIT_TEST_TEE_OFFSET=0\n-\tfi\n-fi\n+start_test_output \"$0\"\n \n # Convenience\n # A regexp to match 5 and 35 hexdigits\n-- \ngitgitgadget\n\n"},{"id":"446777","messageId":"41230100091e3c53fa87516c9db697ad385d5873.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 6/9] test(junit): avoid line feeds in XML attributes","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:11Z","receivedAt":"2022-01-24T19:01:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the test case's output, we do want newline characters, but in the XML\nattributes we do not want them.\n\nHowever, the `xml_attr_encode` function always adds a Line Feed at the\nend (which are then encoded as ` `, even for XML attributes.\n\nThis seems not to faze Azure Pipelines' XML parser, but it still is\nincorrect, so let's fix it.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib-junit.sh | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh\nindex 9d55d74d764..c959183c7e2 100644\n--- a/t/test-lib-junit.sh\n+++ b/t/test-lib-junit.sh\n@@ -50,7 +50,7 @@ finalize_test_case_output () {\n \t\t;;\n \tfailure)\n \t\tjunit_insert=\"\"\n+\t\tjunit_insert=\"$junit_insert $(xml_attr_encode --no-lf \"$1\")\\\">\"\n \t\tjunit_insert=\"$junit_insert $(xml_attr_encode \\\n \t\t\t\"$(if test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n \t\t\t then\n@@ -74,12 +74,12 @@ finalize_test_case_output () {\n \t\tset \"$* (known breakage)\"\n \t\t;;\n \tskip)\n-\t\tmessage=\"$(xml_attr_encode \"$skipped_reason\")\"\n+\t\tmessage=\"$(xml_attr_encode --no-lf \"$skipped_reason\")\"\n \t\tset \"$1\" \" \"\n \t\t;;\n \tesac\n \n-\tjunit_attrs=\"name=\\\"$(xml_attr_encode \"$this_test.$test_count $1\")\\\"\"\n+\tjunit_attrs=\"name=\\\"$(xml_attr_encode --no-lf \"$this_test.$test_count $1\")\\\"\"\n \tshift\n \tjunit_attrs=\"$junit_attrs classname=\\\"$this_test\\\"\"\n \tjunit_attrs=\"$junit_attrs time=\\\"$(test-tool \\\n@@ -122,5 +122,11 @@ write_junit_xml () {\n }\n \n xml_attr_encode () {\n-\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+\tif test \"x$1\" = \"x--no-lf\"\n+\tthen\n+\t\tshift\n+\t\tprintf '%s' \"$*\" | test-tool xml-encode\n+\telse\n+\t\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+\tfi\n }\n-- \ngitgitgadget\n\n"},{"id":"446778","messageId":"992b157588975803faf6747cca94f197611fc5c5.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 9/9] ci: call `finalize_test_case_output` a little later","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:14Z","receivedAt":"2022-01-24T19:02:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe used to call that function already before printing the final verdict.\nHowever, now that we added grouping to the GitHub workflow output, we\nwill want to include even that part in the collapsible group for that\ntest case.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 076bee58c19..1e683ad879b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -732,30 +732,31 @@ trap '{ code=$?; set +x; } 2>/dev/null; exit $code' INT TERM HUP\n # the test_expect_* functions instead.\n \n test_ok_ () {\n-\tfinalize_test_case_output ok \"$@\"\n \ttest_success=$(($test_success + 1))\n \tsay_color \"\" \"ok $test_count - $@\"\n+\tfinalize_test_case_output ok \"$@\"\n }\n \n test_failure_ () {\n-\tfinalize_test_case_output failure \"$@\"\n+\tfailure_label=$1\n \ttest_failure=$(($test_failure + 1))\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || _error_exit\n+\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n }\n \n test_known_broken_ok_ () {\n-\tfinalize_test_case_output fixed \"$@\"\n \ttest_fixed=$(($test_fixed+1))\n \tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n+\tfinalize_test_case_output fixed \"$@\"\n }\n \n test_known_broken_failure_ () {\n-\tfinalize_test_case_output broken \"$@\"\n \ttest_broken=$(($test_broken+1))\n \tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n+\tfinalize_test_case_output broken \"$@\"\n }\n \n test_debug () {\n@@ -1081,10 +1082,10 @@ test_skip () {\n \n \tcase \"$to_skip\" in\n \tt)\n-\t\tfinalize_test_case_output skip \"$@\"\n \n \t\tsay_color skip \"ok $test_count # skip $1 ($skipped_reason)\"\n \t\t: true\n+\t\tfinalize_test_case_output skip \"$@\"\n \t\t;;\n \t*)\n \t\tfalse\n-- \ngitgitgadget\n"},{"id":"446779","messageId":"1a6bd1846bcb061d3dc70870139cb4bc076d2a25.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 8/9] ci: use `--github-workflow-markup` in the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:13Z","receivedAt":"2022-01-24T19:02:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis makes the output easier to digest.\n\nNote: since workflow output currently cannot contain any nested groups\n(see https://github.com/actions/runner/issues/802 for details), we need\nto remove the explicit grouping that would span the entirety of each\nfailed test script.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 4ed8f40ab02..72efdb556ed 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -176,7 +176,7 @@ then\n \t\t\ttest_name=\"${test_exit%.exit}\"\n \t\t\ttest_name=\"${test_name##*/}\"\n \t\t\tprintf \"\\\\e[33m\\\\e[1m=== Failed test: ${test_name} ===\\\\e[m\\\\n\"\n-\t\t\tgroup \"Failed test: $test_name\" cat \"t/test-results/$test_name.out\"\n+\t\t\tcat \"t/test-results/$test_name.markup\"\n \n \t\t\ttrash_dir=\"t/trash directory.$test_name\"\n \t\t\tcp \"t/test-results/$test_name.out\" t/failed-test-artifacts/\n@@ -188,7 +188,7 @@ then\n \tcache_dir=\"$HOME/none\"\n \n \texport GIT_PROVE_OPTS=\"--timer --jobs 10\"\n-\texport GIT_TEST_OPTS=\"--verbose-log -x\"\n+\texport GIT_TEST_OPTS=\"--verbose-log -x --github-workflow-markup\"\n \tMAKEFLAGS=\"$MAKEFLAGS --jobs=10\"\n \ttest windows != \"$CI_OS_NAME\" ||\n \tGIT_TEST_OPTS=\"--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS\"\n-- \ngitgitgadget\n\n"},{"id":"446780","messageId":"98b32630fcd76d591ce64f19314f3081134f541e.1643050574.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.git.1643050574.gitgitgadget@gmail.com","subject":"[PATCH 7/9] ci: optionally mark up output in the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-24T18:56:12Z","receivedAt":"2022-01-24T19:02:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA couple of commands exist to spruce up the output in GitHub workflows:\nhttps://docs.github.com/en/actions/learn-github-actions/workflow-commands-for-github-actions\n\nIn addition to the `::group::\" \"\"\n-\t\twrite_junit_xml=\n-\tfi\n-}\n-\n test_atexit_cleanup=:\n test_atexit_handler () {\n \t# In a succeeding test script 'test_atexit_handler' is invoked\n@@ -1193,7 +1116,7 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tfinalize_junit_xml\n+\tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n \tthen\n@@ -1484,22 +1407,7 @@ fi\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n-if test -n \"$write_junit_xml\"\n-then\n-\tjunit_xml_dir=\"$TEST_OUTPUT_DIRECTORY/out\"\n-\tmkdir -p \"$junit_xml_dir\"\n-\tjunit_xml_base=${0##*/}\n-\tjunit_xml_path=\"$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml\"\n-\tjunit_attrs=\"name=\\\"${junit_xml_base%.sh}\\\"\"\n-\tjunit_attrs=\"$junit_attrs timestamp=\\\"$(TZ=UTC \\\n-\t\tdate +%Y-%m-%dT%H:%M:%S)\\\"\"\n-\twrite_junit_xml --truncate \"\" \" \"\n-\tjunit_suite_start=$(test-tool date getnanos)\n-\tif test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n-\tthen\n-\t\tGIT_TEST_TEE_OFFSET=0\n-\tfi\n-fi\n+start_test_output \"$0\"\n \n # Convenience\n # A regexp to match 5 and 35 hexdigits\n-- \ngitgitgadget\n\n"},{"id":"449914","messageId":"c8b240af749225c1084c722aa150d212e38f2a89.1646130289.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v2.git.1646130289.gitgitgadget@gmail.com","subject":"[PATCH v2 6/9] test(junit): avoid line feeds in XML attributes","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-01T10:24:46Z","receivedAt":"2022-03-01T10:25:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the test case's output, we do want newline characters, but in the XML\nattributes we do not want them.\n\nHowever, the `xml_attr_encode` function always adds a Line Feed at the\nend (which are then encoded as ` `, even for XML attributes.\n\nThis seems not to faze Azure Pipelines' XML parser, but it still is\nincorrect, so let's fix it.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib-junit.sh | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh\nindex 9d55d74d764..c959183c7e2 100644\n--- a/t/test-lib-junit.sh\n+++ b/t/test-lib-junit.sh\n@@ -50,7 +50,7 @@ finalize_test_case_output () {\n \t\t;;\n \tfailure)\n \t\tjunit_insert=\"\"\n+\t\tjunit_insert=\"$junit_insert $(xml_attr_encode --no-lf \"$1\")\\\">\"\n \t\tjunit_insert=\"$junit_insert $(xml_attr_encode \\\n \t\t\t\"$(if test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n \t\t\t then\n@@ -74,12 +74,12 @@ finalize_test_case_output () {\n \t\tset \"$* (known breakage)\"\n \t\t;;\n \tskip)\n-\t\tmessage=\"$(xml_attr_encode \"$skipped_reason\")\"\n+\t\tmessage=\"$(xml_attr_encode --no-lf \"$skipped_reason\")\"\n \t\tset \"$1\" \" \"\n \t\t;;\n \tesac\n \n-\tjunit_attrs=\"name=\\\"$(xml_attr_encode \"$this_test.$test_count $1\")\\\"\"\n+\tjunit_attrs=\"name=\\\"$(xml_attr_encode --no-lf \"$this_test.$test_count $1\")\\\"\"\n \tshift\n \tjunit_attrs=\"$junit_attrs classname=\\\"$this_test\\\"\"\n \tjunit_attrs=\"$junit_attrs time=\\\"$(test-tool \\\n@@ -122,5 +122,11 @@ write_junit_xml () {\n }\n \n xml_attr_encode () {\n-\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+\tif test \"x$1\" = \"x--no-lf\"\n+\tthen\n+\t\tshift\n+\t\tprintf '%s' \"$*\" | test-tool xml-encode\n+\telse\n+\t\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+\tfi\n }\n-- \ngitgitgadget\n\n"},{"id":"449915","messageId":"15f199e810e08701b0c3bb84f1e9737672c100d9.1646130289.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v2.git.1646130289.gitgitgadget@gmail.com","subject":"[PATCH v2 7/9] ci: optionally mark up output in the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-01T10:24:47Z","receivedAt":"2022-03-01T10:25:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA couple of commands exist to spruce up the output in GitHub workflows:\nhttps://docs.github.com/en/actions/learn-github-actions/workflow-commands-for-github-actions\n\nIn addition to the `::group::\" \"\"\n-\t\twrite_junit_xml=\n-\tfi\n-}\n-\n test_atexit_cleanup=:\n test_atexit_handler () {\n \t# In a succeeding test script 'test_atexit_handler' is invoked\n@@ -1248,7 +1171,7 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tfinalize_junit_xml\n+\tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n \tthen\n@@ -1539,22 +1462,7 @@ fi\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n-if test -n \"$write_junit_xml\"\n-then\n-\tjunit_xml_dir=\"$TEST_OUTPUT_DIRECTORY/out\"\n-\tmkdir -p \"$junit_xml_dir\"\n-\tjunit_xml_base=${0##*/}\n-\tjunit_xml_path=\"$junit_xml_dir/TEST-${junit_xml_base%.sh}.xml\"\n-\tjunit_attrs=\"name=\\\"${junit_xml_base%.sh}\\\"\"\n-\tjunit_attrs=\"$junit_attrs timestamp=\\\"$(TZ=UTC \\\n-\t\tdate +%Y-%m-%dT%H:%M:%S)\\\"\"\n-\twrite_junit_xml --truncate \"\" \" \"\n-\tjunit_suite_start=$(test-tool date getnanos)\n-\tif test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n-\tthen\n-\t\tGIT_TEST_TEE_OFFSET=0\n-\tfi\n-fi\n+start_test_output \"$0\"\n \n # Convenience\n # A regexp to match 5 and 35 hexdigits\n-- \ngitgitgadget\n\n"},{"id":"455715","messageId":"30ccd602108fea74d64a5b81e6665fcc7efb6f0a.1653171536.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v3.git.1653171536.gitgitgadget@gmail.com","subject":"[PATCH v3 03/12] test(junit): avoid line feeds in XML attributes","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-21T22:18:47Z","receivedAt":"2022-05-21T22:19:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the test case's output, we do want newline characters, but in the XML\nattributes we do not want them.\n\nHowever, the `xml_attr_encode` function always adds a Line Feed at the\nend (which are then encoded as ` `, even for XML attributes.\n\nThis seems not to faze Azure Pipelines' XML parser, but it still is\nincorrect, so let's fix it.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib-junit.sh | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh\nindex 9d55d74d764..c959183c7e2 100644\n--- a/t/test-lib-junit.sh\n+++ b/t/test-lib-junit.sh\n@@ -50,7 +50,7 @@ finalize_test_case_output () {\n \t\t;;\n \tfailure)\n \t\tjunit_insert=\"\"\n+\t\tjunit_insert=\"$junit_insert $(xml_attr_encode --no-lf \"$1\")\\\">\"\n \t\tjunit_insert=\"$junit_insert $(xml_attr_encode \\\n \t\t\t\"$(if test -n \"$GIT_TEST_TEE_OUTPUT_FILE\"\n \t\t\t then\n@@ -74,12 +74,12 @@ finalize_test_case_output () {\n \t\tset \"$* (known breakage)\"\n \t\t;;\n \tskip)\n-\t\tmessage=\"$(xml_attr_encode \"$skipped_reason\")\"\n+\t\tmessage=\"$(xml_attr_encode --no-lf \"$skipped_reason\")\"\n \t\tset \"$1\" \" \"\n \t\t;;\n \tesac\n \n-\tjunit_attrs=\"name=\\\"$(xml_attr_encode \"$this_test.$test_count $1\")\\\"\"\n+\tjunit_attrs=\"name=\\\"$(xml_attr_encode --no-lf \"$this_test.$test_count $1\")\\\"\"\n \tshift\n \tjunit_attrs=\"$junit_attrs classname=\\\"$this_test\\\"\"\n \tjunit_attrs=\"$junit_attrs time=\\\"$(test-tool \\\n@@ -122,5 +122,11 @@ write_junit_xml () {\n }\n \n xml_attr_encode () {\n-\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+\tif test \"x$1\" = \"x--no-lf\"\n+\tthen\n+\t\tshift\n+\t\tprintf '%s' \"$*\" | test-tool xml-encode\n+\telse\n+\t\tprintf '%s\\n' \"$@\" | test-tool xml-encode\n+\tfi\n }\n-- \ngitgitgadget\n\n"},{"id":"455716","messageId":"8f5b112bd08dcaf63ea764be71a17942c184de0e.1653171536.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v3.git.1653171536.gitgitgadget@gmail.com","subject":"[PATCH v3 04/12] ci/run-build-and-tests: take a more high-level view","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-21T22:18:48Z","receivedAt":"2022-05-21T22:19:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the web UI of GitHub workflows, failed runs are presented with the\njob step that failed auto-expanded. In the current setup, this is not\nhelpful at all because that shows only the output of `prove`, which says\nwhich test failed, but not in what way.\n\nWhat would help understand the reader what went wrong is the verbose\ntest output of the failed test.\n\nThe logs of the failed runs do contain that verbose test output, but it\nis shown in the _next_ step (which is marked as succeeding, and is\ntherefore _not_ auto-expanded). Anyone not intimately familiar with this\nwould completely miss the verbose test output, being left mostly\npuzzled with the test failures.\n\nWe are about to show the failed test cases' output in the _same_ step,\nso that the user has a much easier time to figure out what was going\nwrong.\n\nBut first, we must partially revert the change that tried to improve the\nCI runs by combining the `Makefile` targets to build into a single\n`make` invocation. That might have sounded like a good idea at the time,\nbut it does make it rather impossible for the CI script to determine\nwhether the _build_ failed, or the _tests_. If the tests were run at\nall, that is.\n\nSo let's go back to calling `make` for the build, and call `make test`\nseparately so that we can easily detect that _that_ invocation failed,\nand react appropriately.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/run-build-and-tests.sh | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 280dda7d285..2818b3046ae 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -10,7 +10,7 @@ windows*) cmd //c mklink //j t\\\\.prove \"$(cygpath -aw \"$cache_dir/.prove\")\";;\n *) ln -s \"$cache_dir/.prove\" t/.prove;;\n esac\n \n-export MAKE_TARGETS=\"all test\"\n+run_tests=t\n \n case \"$jobname\" in\n linux-gcc)\n@@ -41,14 +41,15 @@ pedantic)\n \t# Don't run the tests; we only care about whether Git can be\n \t# built.\n \texport DEVOPTS=pedantic\n-\texport MAKE_TARGETS=all\n+\trun_tests=\n \t;;\n esac\n \n-# Any new \"test\" targets should not go after this \"make\", but should\n-# adjust $MAKE_TARGETS. Otherwise compilation-only targets above will\n-# start running tests.\n-make $MAKE_TARGETS\n+make\n+if test -n \"$run_tests\"\n+then\n+\tmake test\n+fi\n check_unignored_build_artifacts\n \n save_good_tree\n-- \ngitgitgadget\n\n"},{"id":"455717","messageId":"417f702a2454316f0bdc5b66eb16751272ff2989.1653171536.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v3.git.1653171536.gitgitgadget@gmail.com","subject":"[PATCH v3 05/12] ci: make it easier to find failed tests' logs in the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-21T22:18:49Z","receivedAt":"2022-05-21T22:19:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen investigating a test failure, the time that matters most is the\ntime it takes from getting aware of the failure to displaying the output\nof the failing test case.\n\nYou currently have to know a lot of implementation details when\ninvestigating test failures in the CI runs. The first step is easy: the\nfailed job is marked quite clearly, but when opening it, the failed step\nis expanded, which in our case is the one running\n`ci/run-build-and-tests.sh`. This step, most notably, only offers a\nhigh-level view of what went wrong: it prints the output of `prove`\nwhich merely tells the reader which test script failed.\n\nThe actually interesting part is in the detailed log of said failed\ntest script. But that log is shown in the CI run's step that runs\n`ci/print-test-failures.sh`. And that step is _not_ expanded in the web\nUI by default. It is even marked as \"successful\", which makes it very\neasy to miss that there is useful information hidden in there.\n\nLet's help the reader by showing the failed tests' detailed logs in the\nstep that is expanded automatically, i.e. directly after the test suite\nfailed.\n\nThis also helps the situation where the _build_ failed and the\n`print-test-failures` step was executed under the assumption that the\n_test suite_ failed, and consequently failed to find any failed tests.\n\nAn alternative way to implement this patch would be to source\n`ci/print-test-failures.sh` in the `handle_test_failures` function to\nshow these logs. However, over the course of the next few commits, we\nwant to introduce some grouping which would be harder to achieve that\nway (for example, we do want a leaner, and colored, preamble for each\nfailed test script, and it would be trickier to accommodate the lack of\nnested groupings in GitHub workflows' output).\n\nSigned-off-by: Johannes Schindelin \n---\n .github/workflows/main.yml | 12 ------------\n ci/lib.sh | 23 +++++++++++++++++++++++\n ci/run-build-and-tests.sh | 3 ++-\n ci/run-test-slice.sh | 3 ++-\n 4 files changed, 27 insertions(+), 14 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex c35200defb9..3fa88b78b6d 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -119,10 +119,6 @@ jobs:\n - name: test\n shell: bash\n run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n- - name: ci/print-test-failures.sh\n- if: failure()\n- shell: bash\n- run: ci/print-test-failures.sh\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v2\n@@ -204,10 +200,6 @@ jobs:\n env:\n NO_SVN_TESTS: 1\n run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n- - name: ci/print-test-failures.sh\n- if: failure()\n- shell: bash\n- run: ci/print-test-failures.sh\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v2\n@@ -261,8 +253,6 @@ jobs:\n - uses: actions/checkout@v2\n - run: ci/install-dependencies.sh\n - run: ci/run-build-and-tests.sh\n- - run: ci/print-test-failures.sh\n- if: failure()\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v2\n@@ -292,8 +282,6 @@ jobs:\n - uses: actions/checkout@v1\n - run: ci/install-docker-dependencies.sh\n - run: ci/run-build-and-tests.sh\n- - run: ci/print-test-failures.sh\n- if: failure()\n - name: Upload failed tests' directories\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n uses: actions/upload-artifact@v1\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex d718f4e386d..65f5188a550 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -78,6 +78,10 @@ check_unignored_build_artifacts () {\n \t}\n }\n \n+handle_failed_tests () {\n+\treturn 1\n+}\n+\n # GitHub Action doesn't set TERM, which is required by tput\n export TERM=${TERM:-dumb}\n \n@@ -123,6 +127,25 @@ then\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n \tCC=\"${CC_PACKAGE:-${CC:-gcc}}\"\n \tDONT_SKIP_TAGS=t\n+\thandle_failed_tests () {\n+\t\tmkdir -p t/failed-test-artifacts\n+\t\techo \"FAILED_TEST_ARTIFACTS=t/failed-test-artifacts\" >>$GITHUB_ENV\n+\n+\t\tfor test_exit in t/test-results/*.exit\n+\t\tdo\n+\t\t\ttest 0 != \"$(cat \"$test_exit\")\" || continue\n+\n+\t\t\ttest_name=\"${test_exit%.exit}\"\n+\t\t\ttest_name=\"${test_name##*/}\"\n+\t\t\tprintf \"\\\\e[33m\\\\e[1m=== Failed test: ${test_name} ===\\\\e[m\\\\n\"\n+\t\t\tcat \"t/test-results/$test_name.out\"\n+\n+\t\t\ttrash_dir=\"t/trash directory.$test_name\"\n+\t\t\tcp \"t/test-results/$test_name.out\" t/failed-test-artifacts/\n+\t\t\ttar czf t/failed-test-artifacts/\"$test_name\".trash.tar.gz \"$trash_dir\"\n+\t\tdone\n+\t\treturn 1\n+\t}\n \n \tcache_dir=\"$HOME/none\"\n \ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 2818b3046ae..1ede75e5556 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -48,7 +48,8 @@ esac\n make\n if test -n \"$run_tests\"\n then\n-\tmake test\n+\tmake test ||\n+\thandle_failed_tests\n fi\n check_unignored_build_artifacts\n \ndiff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh\nindex f8c2c3106a2..63358c23e11 100755\n--- a/ci/run-test-slice.sh\n+++ b/ci/run-test-slice.sh\n@@ -12,6 +12,7 @@ esac\n \n make --quiet -C t T=\"$(cd t &&\n \t./helper/test-tool path-utils slice-tests \"$1\" \"$2\" t[0-9]*.sh |\n-\ttr '\\n' ' ')\"\n+\ttr '\\n' ' ')\" ||\n+handle_failed_tests\n \n check_unignored_build_artifacts\n-- \ngitgitgadget\n\n"},{"id":"455718","messageId":"7d2284314efc0071ad9cc9b8f2559e07310e35b0.1653171536.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v3.git.1653171536.gitgitgadget@gmail.com","subject":"[PATCH v3 06/12] ci/run-build-and-tests: add some structure to the GitHub workflow output","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-21T22:18:50Z","receivedAt":"2022-05-21T22:19:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe current output of Git's GitHub workflow can be quite confusing,\nespecially for contributors new to the project.\n\nTo make it more helpful, let's introduce some collapsible grouping.\nInitially, readers will see the high-level view of what actually\nhappened (did the build fail, or the test suite?). To drill down, the\nrespective group can be expanded.\n\nNote: sadly, workflow output currently cannot contain any nested groups\n(see https://github.com/actions/runner/issues/802 for details),\ntherefore we take pains to ensure to end any previous group before\nstarting a new one.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/lib.sh | 56 ++++++++++++++++++++++++++++++++++-----\n ci/run-build-and-tests.sh | 4 +--\n ci/run-test-slice.sh | 2 +-\n 3 files changed, 52 insertions(+), 10 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 65f5188a550..f8cb79e44f0 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -1,5 +1,50 @@\n # Library of functions shared by all CI scripts\n \n+if test true != \"$GITHUB_ACTIONS\"\n+then\n+\tbegin_group () { :; }\n+\tend_group () { :; }\n+\n+\tgroup () {\n+\t\tshift\n+\t\t\"$@\"\n+\t}\n+\tset -x\n+else\n+\tbegin_group () {\n+\t\tneed_to_end_group=t\n+\t\techo \"::group::$1\" >&2\n+\t\tset -x\n+\t}\n+\n+\tend_group () {\n+\t\ttest -n \"$need_to_end_group\" || return 0\n+\t\tset +x\n+\t\tneed_to_end_group=\n+\t\techo '::endgroup::' >&2\n+\t}\n+\ttrap end_group EXIT\n+\n+\tgroup () {\n+\t\tset +x\n+\t\tbegin_group \"$1\"\n+\t\tshift\n+\t\t\"$@\"\n+\t\tres=$?\n+\t\tend_group\n+\t\treturn $res\n+\t}\n+\n+\tbegin_group \"CI setup\"\n+fi\n+\n+# Set 'exit on error' for all CI scripts to let the caller know that\n+# something went wrong.\n+#\n+# We already enabled tracing executed commands earlier. This helps by showing\n+# how # environment variables are set and and dependencies are installed.\n+set -e\n+\n skip_branch_tip_with_tag () {\n \t# Sometimes, a branch is pushed at the same time the tag that points\n \t# at the same commit as the tip of the branch is pushed, and building\n@@ -88,12 +133,6 @@ export TERM=${TERM:-dumb}\n # Clear MAKEFLAGS that may come from the outside world.\n export MAKEFLAGS=\n \n-# Set 'exit on error' for all CI scripts to let the caller know that\n-# something went wrong.\n-# Set tracing executed commands, primarily setting environment variables\n-# and installing dependencies.\n-set -ex\n-\n if test -n \"$SYSTEM_COLLECTIONURI\" || test -n \"$SYSTEM_TASKDEFINITIONSURI\"\n then\n \tCI_TYPE=azure-pipelines\n@@ -138,7 +177,7 @@ then\n \t\t\ttest_name=\"${test_exit%.exit}\"\n \t\t\ttest_name=\"${test_name##*/}\"\n \t\t\tprintf \"\\\\e[33m\\\\e[1m=== Failed test: ${test_name} ===\\\\e[m\\\\n\"\n-\t\t\tcat \"t/test-results/$test_name.out\"\n+\t\t\tgroup \"Failed test: $test_name\" cat \"t/test-results/$test_name.out\"\n \n \t\t\ttrash_dir=\"t/trash directory.$test_name\"\n \t\t\tcp \"t/test-results/$test_name.out\" t/failed-test-artifacts/\n@@ -233,3 +272,6 @@ linux-leaks)\n esac\n \n MAKEFLAGS=\"$MAKEFLAGS CC=${CC:-cc}\"\n+\n+end_group\n+set -x\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 1ede75e5556..7abfa00adc0 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -45,10 +45,10 @@ pedantic)\n \t;;\n esac\n \n-make\n+group Build make\n if test -n \"$run_tests\"\n then\n-\tmake test ||\n+\tgroup \"Run tests\" make test ||\n \thandle_failed_tests\n fi\n check_unignored_build_artifacts\ndiff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh\nindex 63358c23e11..a3c67956a8d 100755\n--- a/ci/run-test-slice.sh\n+++ b/ci/run-test-slice.sh\n@@ -10,7 +10,7 @@ windows*) cmd //c mklink //j t\\\\.prove \"$(cygpath -aw \"$cache_dir/.prove\")\";;\n *) ln -s \"$cache_dir/.prove\" t/.prove;;\n esac\n \n-make --quiet -C t T=\"$(cd t &&\n+group \"Run tests\" make --quiet -C t T=\"$(cd t &&\n \t./helper/test-tool path-utils slice-tests \"$1\" \"$2\" t[0-9]*.sh |\n \ttr '\\n' ' ')\" ||\n handle_failed_tests\n-- \ngitgitgadget\n\n"},{"id":"455719","messageId":"98059b94a881485f763efc54346e8b19103ee64b.1653171536.git.gitgitgadget@gmail.com","threadId":"57297","inReplyTo":"pull.1117.v3.git.1653171536.gitgitgadget@gmail.com","subject":"[PATCH v3 07/12] ci: optionally mark up output in the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-21T22:18:51Z","receivedAt":"2022-05-21T22:19:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA couple of commands exist to spruce up the output in GitHub workflows:\nhttps://docs.github.com/en/actions/learn-github-actions/workflow-commands-for-github-actions\n\nIn addition to the `::group::