On Mon, Jan 24, 2022 at 3:02 PM Johannes Schindelin via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > In the web UI of GitHub workflows, failed runs are presented with the > job step that failed auto-expanded. In the current setup, this is not > helpful at all because that shows only the output of `prove`, which says > which test failed, but not in what way. > > What would help understand the reader what went wrong is the verbose > test output of the failed test. > > The logs of the failed runs do contain that verbose test output, but it > is shown in the _next_ step (which is marked as succeeding, and is > therefore _not_ auto-expanded). Anyone not intimately familiar with this > would completely miss the verbose test output, being left mostly > puzzled with the test failures. > > We are about to show the failed test cases' output in the _same_ step, > so that the user has a much easier time to figure out what was going > wrong. > > But first, we must partially revert the change that tried to improve the > CI runs by combining the `Makefile` targets to build into a single > `make` invocation. That might have sounded like a good idea at the time, > but it does make it rather impossible for the CI script to determine > whether the _build_ failed, or the _tests_. If the tests were run at > all, that is. > > So let's go back to calling `make` for the build, and call `make test` > separately so that we can easily detect that _that_ invocation failed, > and react appropriately. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> > --- > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh > @@ -10,7 +10,7 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";; > -export MAKE_TARGETS="all test" > +run_tests=t > > case "$jobname" in > linux-gcc) > @@ -41,14 +41,18 @@ pedantic) > # Don't run the tests; we only care about whether Git can be > # built. > export DEVOPTS=pedantic > - export MAKE_TARGETS=all > + run_tests= > ;; > esac > > # Any new "test" targets should not go after this "make", but should > # adjust $MAKE_TARGETS. Otherwise compilation-only targets above will > # start running tests. > -make $MAKE_TARGETS The comment talking about MAKE_TARGETS seems out of date now that MAKE_TARGETS has been removed from this script. > +make > +if test -n "$run_tests" > +then > + make test > +fi > check_unignored_build_artifacts This changes behavior, doesn't it? Wth the original "make all test", if the `all` target failed, then the `test` target would not be invoked. However, with the revised code, `make test` is invoked even if `make all` fails. Is that behavior change significant? Do we care about it?
