mbox series

[00/25] CI: run "make [test]" directly, use $GITHUB_ENV

Message ID cover-00.25-00000000000-20220221T143936Z-avarab@gmail.com (mailing list archive)
Headers show
Series CI: run "make [test]" directly, use $GITHUB_ENV | expand

Message

Ævar Arnfjörð Bjarmason Feb. 21, 2022, 2:46 p.m. UTC
== Applying this

Merging this has a trivial conflct with "seen"'s
js/use-builtin-add-i. The
s/GIT_TEST_ADD_I_USE_BUILTIN=1/GIT_TEST_ADD_I_USE_BUILTIN=0/g" there
needs to be correspondingly changed (using "setenv", no "export") in
ci/lib.sh

== Summary

This series makes the CI less of a special-case by getting rid of
wrapper scripts like "ci/run-build-and-tests.sh" in favor of simply
running "make" or "make test" directly.

It also has changes that make us do less special-ness in
CI. E.g. there's now a "make ci-static-analysis" target that can be
run locally to do what "ci/run-static-analysis.sh" did before. The CI
just invokes that new "make" target, and the
"ci/run-static-analysis.sh" script has been "git rm'd".

The 12/25 here has a more detailed summary[1], but basically the
"ci/lib.sh" is now a script run directly from an early CI step within
a given job. That script runs sets the environment variables needed in
$GITHUB_ENV (what GitHub CI uses to carry forward the environment
between steps).

== Details

This series conflicts with and is a proposed alternative to Johannes's
proposed changes to teach "ci/lib.sh" GitHub specific syntax to
"group" certain parts of the output[2]. That series proposes to keep
the "ci/run-build-and-tests.sh" and similar scripts, but to teach them
to emit markers indicating when the "build" part commences, then
"test" etc.

I think the approach offered here is better as elaborated on in
12/25[1], especially because as Johannes notes[3] we only have one
level of "grouping" within a "step" available to us. In his version
using it for "build" v.s. "test" precludes being able to group parts
of that "build" or "test" output in the future.

The following compares CI output between the three with a
change-on-top which caused a compilation error ("this" is this series,
"js" is Johannes's) when clicking on the "linux-gcc" failure:

  master: https://github.com/avar/git/runs/5274251909?check_suite_focus=true
  this: https://github.com/avar/git/runs/5274464670?check_suite_focus=true
  js: https://github.com/avar/git/runs/5272239403?check_suite_focus=true

  Here we went from 93 lines of output on "master" to 47 in "this"
  (107 in "js"). Note also how in "this" we can:

  - Expand the "Run make" to get the "MAKEFLAGS" and other variables
    used in the step. That's now a 3-line summary instead of the
    first 50 lines being the old "ci/lib.sh" "set -x" output.

  - Because "make" failed, we have an elided "make test" step that's
    not being run below. We can thus see what steps our failure caused
    us to skip.

  - Unlike the "js" version we'll show the compilation error by
    default (the "js" version is grouped and needs to be expanded),
    but our output is now brief enough that that's no longer
    surrounded by other irrelevant output.

  - Unlike the "js" version there is no "CI setup" group within each
    step. That work is in the earlier "ci/lib.sh --build" step
    instead, which sets the config for all subsequent steps.

For other output changes look at the difference between "master" and
"this" at:

  master: https://github.com/git/git/actions/runs/1866786595
  this: https://github.com/avar/git/actions/runs/1876270588

Not explicitly covered in the summary above is that various other
parts of ci/* were doing the same sort of within-step setup, but now
do so via cross-step passing of variables via $GITHUB_ENV. E.g. on
"master" the test slices for the Windows tests have a lot of verbosity
before they get to running the test itself:

    https://github.com/git/git/runs/5254267914?check_suite_focus=true#step:5:56

In the "js" version the test output is hidden ("grouped"), but we've
got the same amount of verbosity by default:

    https://github.com/gitgitgadget/git/runs/4937491771?check_suite_focus=true#step:5:67

Whereas in "this" version that verbosity is in the preceding "select
tests" step, with the "test" step showing only the relevant end-state
of the "$T" variable we'll use in the Makefile (hidden by default,
expanded in this link):

    https://github.com/avar/git/runs/5274558869?check_suite_focus=true#step:7:13

This series does not attempt to replace the use of the "group" output
used for the "ok" or "not ok" portion of tests in Johannes's
series. When a test fails the output in this series (sans config
discovery not being repeated, and now summarized in the $GITHUB_ENV
drop-down) is substantially the same as on "master".

My summary at [4] goes into other overlap & non-overlap between the
two. I think using the "group" output for those purposes might be
useful, although I left some feedback on [5] with problems in the
current "js" implementation.

I do think any such changes should follow behind this series, as doing
that sort of grouping once we can effectively free up an extra "group"
level (by peeling those off into "steps", as is done here) would be
much more useful.

1. https://lore.kernel.org/git/patch-12.25-dfd823f2e7d-20220221T122605Z-avarab@gmail.com
2. https://lore.kernel.org/git/pull.1117.git.1643050574.gitgitgadget@gmail.com/
3. https://lore.kernel.org/git/9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@gmail.com/
4. https://lore.kernel.org/git/220127.86ilu5cdnf.gmgdl@evledraar.gmail.com/
5. https://lore.kernel.org/git/220126.86sftbfjl4.gmgdl@evledraar.gmail.com/

Ævar Arnfjörð Bjarmason (25):
  CI: run "set -ex" early in ci/lib.sh
  CI: make "$jobname" explicit, remove fallback
  CI: remove more dead Travis CI support
  CI: remove dead "tree skipping" code
  CI: remove unused Azure ci/* code
  CI: don't have "git grep" invoke a pager in tree content check
  CI: have "static-analysis" run a "make ci-static-analysis" target
  CI: have "static-analysis" run "check-builtins", not "documentation"
  CI: move p4 and git-lfs variables to ci/install-dependencies.sh
  CI: consistently use "export" in ci/lib.sh
  CI: export variables via a wrapper
  CI: remove "run-build-and-tests.sh", run "make [test]" directly
  CI: check ignored unignored build artifacts in "win[+VS] build" too
  CI: invoke "make artifacts-tar" directly in windows-build
  CI: split up and reduce "ci/test-documentation.sh"
  CI: combine ci/install{,-docker}-dependencies.sh
  CI: move "env" definitions into ci/lib.sh
  ci/run-test-slice.sh: replace shelling out with "echo"
  CI: pre-select test slice in Windows & VS tests
  CI: only invoke ci/lib.sh as "steps" in main.yml
  CI: narrow down variable definitions in --build and --test
  CI: add more variables to MAKEFLAGS, except under vs-build
  CI: stop over-setting the $CC variable
  CI: set PYTHON_PATH setting for osx-{clang,gcc} into "$jobname" case
  CI: don't use "set -x" in "ci/lib.sh" output

 .github/workflows/main.yml            | 100 +++++---
 Makefile                              |  32 ++-
 ci/check-directional-formatting.bash  |   2 +-
 ci/check-unignored-build-artifacts.sh |  20 ++
 ci/install-dependencies.sh            |  53 ++++-
 ci/install-docker-dependencies.sh     |  22 --
 ci/lib-ci-type.sh                     |  10 +
 ci/lib-tput.sh                        |   2 +
 ci/lib.sh                             | 315 ++++++++++++--------------
 ci/make-test-artifacts.sh             |  12 -
 ci/mount-fileshare.sh                 |  25 --
 ci/print-test-failures.sh             |  16 +-
 ci/run-build-and-tests.sh             |  54 -----
 ci/run-docker-build.sh                |  66 ------
 ci/run-docker.sh                      |  47 ----
 ci/run-static-analysis.sh             |  32 ---
 ci/run-test-slice.sh                  |  17 --
 ci/select-test-slice.sh               |  10 +
 ci/test-documentation.sh              |  37 +--
 ci/util/extract-trash-dirs.sh         |  50 ----
 20 files changed, 345 insertions(+), 577 deletions(-)
 create mode 100755 ci/check-unignored-build-artifacts.sh
 delete mode 100755 ci/install-docker-dependencies.sh
 create mode 100644 ci/lib-ci-type.sh
 create mode 100644 ci/lib-tput.sh
 delete mode 100755 ci/make-test-artifacts.sh
 delete mode 100755 ci/mount-fileshare.sh
 delete mode 100755 ci/run-build-and-tests.sh
 delete mode 100755 ci/run-docker-build.sh
 delete mode 100755 ci/run-docker.sh
 delete mode 100755 ci/run-static-analysis.sh
 delete mode 100755 ci/run-test-slice.sh
 create mode 100755 ci/select-test-slice.sh
 delete mode 100755 ci/util/extract-trash-dirs.sh

Comments

Phillip Wood Feb. 23, 2022, 1:55 p.m. UTC | #1
Hi Ævar

On 21/02/2022 14:46, Ævar Arnfjörð Bjarmason wrote:
 > [...]
> == Details
> 
> This series conflicts with and is a proposed alternative to Johannes's
> proposed changes to teach "ci/lib.sh" GitHub specific syntax to
> "group" certain parts of the output[2]. That series proposes to keep
> the "ci/run-build-and-tests.sh" and similar scripts, but to teach them
> to emit markers indicating when the "build" part commences, then
> "test" etc.
> 
> I think the approach offered here is better as elaborated on in
> 12/25[1], especially because as Johannes notes[3] we only have one
> level of "grouping" within a "step" available to us. In his version
> using it for "build" v.s. "test" precludes being able to group parts
> of that "build" or "test" output in the future.
> 
> The following compares CI output between the three with a
> change-on-top which caused a compilation error ("this" is this series,
> "js" is Johannes's) when clicking on the "linux-gcc" failure:
> 
>    master: https://github.com/avar/git/runs/5274251909?check_suite_focus=true
>    this: https://github.com/avar/git/runs/5274464670?check_suite_focus=true
>    js: https://github.com/avar/git/runs/5272239403?check_suite_focus=true
> 
>    Here we went from 93 lines of output on "master" to 47 in "this"
>    (107 in "js"). Note also how in "this" we can:
> 
>    - Expand the "Run make" to get the "MAKEFLAGS" and other variables
>      used in the step. That's now a 3-line summary instead of the
>      first 50 lines being the old "ci/lib.sh" "set -x" output.
> 
>    - Because "make" failed, we have an elided "make test" step that's
>      not being run below. We can thus see what steps our failure caused
>      us to skip.
> 
>    - Unlike the "js" version we'll show the compilation error by
>      default (the "js" version is grouped and needs to be expanded),
>      but our output is now brief enough that that's no longer
>      surrounded by other irrelevant output.
> 
>    - Unlike the "js" version there is no "CI setup" group within each
>      step. That work is in the earlier "ci/lib.sh --build" step
>      instead, which sets the config for all subsequent steps.

I can see that showing compilation errors without having to expand 
anything is useful but in my experience they are relatively rare 
compared to test failures. If I understand the rest of the message 
correctly we're left with having to expand print-test-failures and 
searching for "not ok" to find out what went wrong with this series.

> For other output changes look at the difference between "master" and
> "this" at:
> 
>    master: https://github.com/git/git/actions/runs/1866786595
>    this: https://github.com/avar/git/actions/runs/1876270588
> 
> Not explicitly covered in the summary above is that various other
> parts of ci/* were doing the same sort of within-step setup, but now
> do so via cross-step passing of variables via $GITHUB_ENV. E.g. on
> "master" the test slices for the Windows tests have a lot of verbosity
> before they get to running the test itself:
> 
>      https://github.com/git/git/runs/5254267914?check_suite_focus=true#step:5:56
> 
> In the "js" version the test output is hidden ("grouped"), but we've
> got the same amount of verbosity by default:
> 
>      https://github.com/gitgitgadget/git/runs/4937491771?check_suite_focus=true#step:5:67
> 
> Whereas in "this" version that verbosity is in the preceding "select
> tests" step, with the "test" step showing only the relevant end-state
> of the "$T" variable we'll use in the Makefile (hidden by default,
> expanded in this link):
> 
>      https://github.com/avar/git/runs/5274558869?check_suite_focus=true#step:7:13

All I can see in the "$T" variable I have to scroll to get the prove 
output on screen


> This series does not attempt to replace the use of the "group" output
> used for the "ok" or "not ok" portion of tests in Johannes's
> series. When a test fails the output in this series (sans config
> discovery not being repeated, and now summarized in the $GITHUB_ENV
> drop-down) is substantially the same as on "master".
> 
> My summary at [4] goes into other overlap & non-overlap between the
> two. I think using the "group" output for those purposes might be
> useful, although I left some feedback on [5] with problems in the
> current "js" implementation.
> 
> I do think any such changes should follow behind this series, as doing
> that sort of grouping once we can effectively free up an extra "group"
> level (by peeling those off into "steps", as is done here) would be
> much more useful.

I can see the attraction of being able to use make directly in the ci 
from an implementation point of view but from the point of view of 
someone trying to investigate a test failure then unless I've 
misunderstood I don't think this series improves the current situation. 
Why can't you build on top of Dscho's series that makes it easier to see 
the output of the failed tests?

Best Wishes

Phillip

> 1. https://lore.kernel.org/git/patch-12.25-dfd823f2e7d-20220221T122605Z-avarab@gmail.com
> 2. https://lore.kernel.org/git/pull.1117.git.1643050574.gitgitgadget@gmail.com/
> 3. https://lore.kernel.org/git/9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@gmail.com/
> 4. https://lore.kernel.org/git/220127.86ilu5cdnf.gmgdl@evledraar.gmail.com/
> 5. https://lore.kernel.org/git/220126.86sftbfjl4.gmgdl@evledraar.gmail.com/
> 
> Ævar Arnfjörð Bjarmason (25):
>    CI: run "set -ex" early in ci/lib.sh
>    CI: make "$jobname" explicit, remove fallback
>    CI: remove more dead Travis CI support
>    CI: remove dead "tree skipping" code
>    CI: remove unused Azure ci/* code
>    CI: don't have "git grep" invoke a pager in tree content check
>    CI: have "static-analysis" run a "make ci-static-analysis" target
>    CI: have "static-analysis" run "check-builtins", not "documentation"
>    CI: move p4 and git-lfs variables to ci/install-dependencies.sh
>    CI: consistently use "export" in ci/lib.sh
>    CI: export variables via a wrapper
>    CI: remove "run-build-and-tests.sh", run "make [test]" directly
>    CI: check ignored unignored build artifacts in "win[+VS] build" too
>    CI: invoke "make artifacts-tar" directly in windows-build
>    CI: split up and reduce "ci/test-documentation.sh"
>    CI: combine ci/install{,-docker}-dependencies.sh
>    CI: move "env" definitions into ci/lib.sh
>    ci/run-test-slice.sh: replace shelling out with "echo"
>    CI: pre-select test slice in Windows & VS tests
>    CI: only invoke ci/lib.sh as "steps" in main.yml
>    CI: narrow down variable definitions in --build and --test
>    CI: add more variables to MAKEFLAGS, except under vs-build
>    CI: stop over-setting the $CC variable
>    CI: set PYTHON_PATH setting for osx-{clang,gcc} into "$jobname" case
>    CI: don't use "set -x" in "ci/lib.sh" output
> 
>   .github/workflows/main.yml            | 100 +++++---
>   Makefile                              |  32 ++-
>   ci/check-directional-formatting.bash  |   2 +-
>   ci/check-unignored-build-artifacts.sh |  20 ++
>   ci/install-dependencies.sh            |  53 ++++-
>   ci/install-docker-dependencies.sh     |  22 --
>   ci/lib-ci-type.sh                     |  10 +
>   ci/lib-tput.sh                        |   2 +
>   ci/lib.sh                             | 315 ++++++++++++--------------
>   ci/make-test-artifacts.sh             |  12 -
>   ci/mount-fileshare.sh                 |  25 --
>   ci/print-test-failures.sh             |  16 +-
>   ci/run-build-and-tests.sh             |  54 -----
>   ci/run-docker-build.sh                |  66 ------
>   ci/run-docker.sh                      |  47 ----
>   ci/run-static-analysis.sh             |  32 ---
>   ci/run-test-slice.sh                  |  17 --
>   ci/select-test-slice.sh               |  10 +
>   ci/test-documentation.sh              |  37 +--
>   ci/util/extract-trash-dirs.sh         |  50 ----
>   20 files changed, 345 insertions(+), 577 deletions(-)
>   create mode 100755 ci/check-unignored-build-artifacts.sh
>   delete mode 100755 ci/install-docker-dependencies.sh
>   create mode 100644 ci/lib-ci-type.sh
>   create mode 100644 ci/lib-tput.sh
>   delete mode 100755 ci/make-test-artifacts.sh
>   delete mode 100755 ci/mount-fileshare.sh
>   delete mode 100755 ci/run-build-and-tests.sh
>   delete mode 100755 ci/run-docker-build.sh
>   delete mode 100755 ci/run-docker.sh
>   delete mode 100755 ci/run-static-analysis.sh
>   delete mode 100755 ci/run-test-slice.sh
>   create mode 100755 ci/select-test-slice.sh
>   delete mode 100755 ci/util/extract-trash-dirs.sh
>
Junio C Hamano Feb. 23, 2022, 8:12 p.m. UTC | #2
Phillip Wood <phillip.wood123@gmail.com> writes:

> I can see that showing compilation errors without having to expand
> anything is useful but in my experience they are relatively rare 
> compared to test failures. If I understand the rest of the message
> correctly we're left with having to expand print-test-failures and 
> searching for "not ok" to find out what went wrong with this series.

It matches my experience that most of the time I have to look for
test errors, not compilation errors.  Having to expand the other
group and looking for "not ok" is something I've got accustomed to,
so if that part improves, that would be great, but if that part does
not become worse, then that is OK, at least to me ;-)
Ævar Arnfjörð Bjarmason Feb. 23, 2022, 9:18 p.m. UTC | #3
On Wed, Feb 23 2022, Phillip Wood wrote:

> On 21/02/2022 14:46, Ævar Arnfjörð Bjarmason wrote:
>> [...]
>> == Details
>> This series conflicts with and is a proposed alternative to
>> Johannes's
>> proposed changes to teach "ci/lib.sh" GitHub specific syntax to
>> "group" certain parts of the output[2]. That series proposes to keep
>> the "ci/run-build-and-tests.sh" and similar scripts, but to teach them
>> to emit markers indicating when the "build" part commences, then
>> "test" etc.
>> I think the approach offered here is better as elaborated on in
>> 12/25[1], especially because as Johannes notes[3] we only have one
>> level of "grouping" within a "step" available to us. In his version
>> using it for "build" v.s. "test" precludes being able to group parts
>> of that "build" or "test" output in the future.
>> The following compares CI output between the three with a
>> change-on-top which caused a compilation error ("this" is this series,
>> "js" is Johannes's) when clicking on the "linux-gcc" failure:
>>    master:
>> https://github.com/avar/git/runs/5274251909?check_suite_focus=true
>>    this: https://github.com/avar/git/runs/5274464670?check_suite_focus=true
>>    js: https://github.com/avar/git/runs/5272239403?check_suite_focus=true
>>    Here we went from 93 lines of output on "master" to 47 in "this"
>>    (107 in "js"). Note also how in "this" we can:
>>    - Expand the "Run make" to get the "MAKEFLAGS" and other
>> variables
>>      used in the step. That's now a 3-line summary instead of the
>>      first 50 lines being the old "ci/lib.sh" "set -x" output.
>>    - Because "make" failed, we have an elided "make test" step
>> that's
>>      not being run below. We can thus see what steps our failure caused
>>      us to skip.
>>    - Unlike the "js" version we'll show the compilation error by
>>      default (the "js" version is grouped and needs to be expanded),
>>      but our output is now brief enough that that's no longer
>>      surrounded by other irrelevant output.
>>    - Unlike the "js" version there is no "CI setup" group within
>> each
>>      step. That work is in the earlier "ci/lib.sh --build" step
>>      instead, which sets the config for all subsequent steps.
>
> I can see that showing compilation errors without having to expand
> anything is useful but in my experience they are relatively rare 
> compared to test failures. If I understand the rest of the message
> correctly we're left with having to expand print-test-failures and 
> searching for "not ok" to find out what went wrong with this series.

Yes, it's a non-goal of this series to directly improve or change the
"prove" output, or to replace or change the functioning of
ci/print-test-failures.sh.

I think that's also a thing worth doing, it's just not what I'm aiming
for here. I.e. the goals of this & Johannes's series is only partial.

The above summary is comparing the parts of the two where those goals do
overlap, i.e. it's rougly comparable to patches 1-4/9 of Johannes's
series. His 5-9/9 are (mostly) orthagonal modifications to t/test-lib.sh
etc.

>> For other output changes look at the difference between "master" and
>> "this" at:
>>    master: https://github.com/git/git/actions/runs/1866786595
>>    this: https://github.com/avar/git/actions/runs/1876270588
>> Not explicitly covered in the summary above is that various other
>> parts of ci/* were doing the same sort of within-step setup, but now
>> do so via cross-step passing of variables via $GITHUB_ENV. E.g. on
>> "master" the test slices for the Windows tests have a lot of verbosity
>> before they get to running the test itself:
>>      https://github.com/git/git/runs/5254267914?check_suite_focus=true#step:5:56
>> In the "js" version the test output is hidden ("grouped"), but we've
>> got the same amount of verbosity by default:
>>      https://github.com/gitgitgadget/git/runs/4937491771?check_suite_focus=true#step:5:67
>> Whereas in "this" version that verbosity is in the preceding "select
>> tests" step, with the "test" step showing only the relevant end-state
>> of the "$T" variable we'll use in the Makefile (hidden by default,
>> expanded in this link):
>>      https://github.com/avar/git/runs/5274558869?check_suite_focus=true#step:7:13
>
> All I can see in the "$T" variable I have to scroll to get the prove
> output on screen

Yes, what that link is showing you is that for the test run the
variables that control the full set of variables that control the test
run are now prominently displayed.

All of the output etc. after that isn't changed.

The point of those comparisons is to draw your eyes to the part that
*is* changed v.s. master, and how using that variable passing mechanism
is making things related to that simpler & less verbose.

>> This series does not attempt to replace the use of the "group" output
>> used for the "ok" or "not ok" portion of tests in Johannes's
>> series. When a test fails the output in this series (sans config
>> discovery not being repeated, and now summarized in the $GITHUB_ENV
>> drop-down) is substantially the same as on "master".
>> My summary at [4] goes into other overlap & non-overlap between the
>> two. I think using the "group" output for those purposes might be
>> useful, although I left some feedback on [5] with problems in the
>> current "js" implementation.
>> I do think any such changes should follow behind this series, as
>> doing
>> that sort of grouping once we can effectively free up an extra "group"
>> level (by peeling those off into "steps", as is done here) would be
>> much more useful.
>
> I can see the attraction of being able to use make directly in the ci
> from an implementation point of view but from the point of view of 
> someone trying to investigate a test failure then unless I've
> misunderstood I don't think this series improves the current
> situation.

Johannes's series is making use of GitHub's group output with changes to
ci/lib.sh to e.g. distinguish the "make" phase from "make test".

This series shows that's not something that's needed to "group" those
two.

Instead we can set set the variables and other context that ci/lib.sh
currently sets at the start of every "step" in an earlier "step" in a
way that spans later steps.

Those later steps are then a light "make" or "make test", and because
steps are their own "group" it accomplishes the same goals for grouping
that part of the output, without needing any CI-specific syntax in ci/*.

Further, if we ever wanted to make use of such syntax for the
"ci/lib.sh" part we could do that on top of this series for
e.g. "grouping" individual errors that "make" emits.

Doing that wouldn't be possible with Johannes's approach, since there's
only one "group" level, you can't nest them. So using it for "build" and
"test" means you've already used up your one-level.

It's also a lot simpler, the resulting code size for the two in terms of
*.sh code in ci/ is:

    $ git grep . avar/ci-unroll-make-commands-to-ci-recipe 'ci/**.sh'|wc -l
    422
    $ git grep . pr-1117/dscho/use-grouping-in-ci-v1 'ci/**.sh'|wc -l
    721

> Why can't you build on top of Dscho's series that makes it
> easier to see the output of the failed tests?

Because building on top of it would mean adding a bunch of code and
complexity to ci/* to accomplish a goal that this series shows that we
can equally get without that complexity and added code, as the diffstat
etc. shows.

What we do about t/test-lib.sh, "prove" etc. emitting summary output
about tests is then a mostly orthagonal change on top of that.

"Mostly" except insofar that Johannes's series notes that he's left with
the limitation[3] that "sadly, workflow output currently cannot contain
any nested groups".

Of course that's also true after this series (that GitHub CI feature
will be the same).

But the difference is that since we won't have used the "group" for
"make" and "make test" (as it's "kicked up" to the "step" level) a
series to add "group"-ing to the test output wouldn't have the group for
an individual test failure at the same level as "make test".

>> 1. https://lore.kernel.org/git/patch-12.25-dfd823f2e7d-20220221T122605Z-avarab@gmail.com
>> 2. https://lore.kernel.org/git/pull.1117.git.1643050574.gitgitgadget@gmail.com/
>> 3. https://lore.kernel.org/git/9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@gmail.com/
>> 4. https://lore.kernel.org/git/220127.86ilu5cdnf.gmgdl@evledraar.gmail.com/
>> 5. https://lore.kernel.org/git/220126.86sftbfjl4.gmgdl@evledraar.gmail.com/
>> Ævar Arnfjörð Bjarmason (25):
>>    CI: run "set -ex" early in ci/lib.sh
>>    CI: make "$jobname" explicit, remove fallback
>>    CI: remove more dead Travis CI support
>>    CI: remove dead "tree skipping" code
>>    CI: remove unused Azure ci/* code
>>    CI: don't have "git grep" invoke a pager in tree content check
>>    CI: have "static-analysis" run a "make ci-static-analysis" target
>>    CI: have "static-analysis" run "check-builtins", not "documentation"
>>    CI: move p4 and git-lfs variables to ci/install-dependencies.sh
>>    CI: consistently use "export" in ci/lib.sh
>>    CI: export variables via a wrapper
>>    CI: remove "run-build-and-tests.sh", run "make [test]" directly
>>    CI: check ignored unignored build artifacts in "win[+VS] build" too
>>    CI: invoke "make artifacts-tar" directly in windows-build
>>    CI: split up and reduce "ci/test-documentation.sh"
>>    CI: combine ci/install{,-docker}-dependencies.sh
>>    CI: move "env" definitions into ci/lib.sh
>>    ci/run-test-slice.sh: replace shelling out with "echo"
>>    CI: pre-select test slice in Windows & VS tests
>>    CI: only invoke ci/lib.sh as "steps" in main.yml
>>    CI: narrow down variable definitions in --build and --test
>>    CI: add more variables to MAKEFLAGS, except under vs-build
>>    CI: stop over-setting the $CC variable
>>    CI: set PYTHON_PATH setting for osx-{clang,gcc} into "$jobname" case
>>    CI: don't use "set -x" in "ci/lib.sh" output
>>   .github/workflows/main.yml            | 100 +++++---
>>   Makefile                              |  32 ++-
>>   ci/check-directional-formatting.bash  |   2 +-
>>   ci/check-unignored-build-artifacts.sh |  20 ++
>>   ci/install-dependencies.sh            |  53 ++++-
>>   ci/install-docker-dependencies.sh     |  22 --
>>   ci/lib-ci-type.sh                     |  10 +
>>   ci/lib-tput.sh                        |   2 +
>>   ci/lib.sh                             | 315 ++++++++++++--------------
>>   ci/make-test-artifacts.sh             |  12 -
>>   ci/mount-fileshare.sh                 |  25 --
>>   ci/print-test-failures.sh             |  16 +-
>>   ci/run-build-and-tests.sh             |  54 -----
>>   ci/run-docker-build.sh                |  66 ------
>>   ci/run-docker.sh                      |  47 ----
>>   ci/run-static-analysis.sh             |  32 ---
>>   ci/run-test-slice.sh                  |  17 --
>>   ci/select-test-slice.sh               |  10 +
>>   ci/test-documentation.sh              |  37 +--
>>   ci/util/extract-trash-dirs.sh         |  50 ----
>>   20 files changed, 345 insertions(+), 577 deletions(-)
>>   create mode 100755 ci/check-unignored-build-artifacts.sh
>>   delete mode 100755 ci/install-docker-dependencies.sh
>>   create mode 100644 ci/lib-ci-type.sh
>>   create mode 100644 ci/lib-tput.sh
>>   delete mode 100755 ci/make-test-artifacts.sh
>>   delete mode 100755 ci/mount-fileshare.sh
>>   delete mode 100755 ci/run-build-and-tests.sh
>>   delete mode 100755 ci/run-docker-build.sh
>>   delete mode 100755 ci/run-docker.sh
>>   delete mode 100755 ci/run-static-analysis.sh
>>   delete mode 100755 ci/run-test-slice.sh
>>   create mode 100755 ci/select-test-slice.sh
>>   delete mode 100755 ci/util/extract-trash-dirs.sh
>>
Johannes Schindelin March 9, 2022, 10:10 p.m. UTC | #4
Hi Ævar,

On Wed, 23 Feb 2022, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Feb 23 2022, Phillip Wood wrote:
>
> > If I understand the rest of the message correctly we're left with
> > having to expand print-test-failures and searching for "not ok" to
> > find out what went wrong with this series.
>
> Yes, it's a non-goal of this series to directly improve or change the
> "prove" output, or to replace or change the functioning of
> ci/print-test-failures.sh.

Since it is explicitly _not_ the goal of this series, but of the series I
offered for review in
https://lore.kernel.org/git/pull.1117.v2.git.1646130289.gitgitgadget@gmail.com/
(with which this here patch series conflicts rather intentionally), I
suggest that your patch series can wait a little longer and build on top
of mine.

We do have the rule in the Git project to try to avoid conflicting with
patch series that are already in flight. It just makes the lives of all
involved parties so much easier.

Thanks,
Johannes