Re: [PATCH v3 13/29] CI: remove "run-build-and-tests.sh", run "make [test]" directly

[Eric Sunshine <sunshine@sunshineco.com>]

On 4/13/22 3:51 PM, Ævar Arnfjörð Bjarmason wrote:
> Remove the already thin "ci/run-build-and-tests.sh" wrapper and
> instead make the CI run "make" or "make test" directly. By doing this
> we'll be able to easily see at a glance whether our failure was in the
> compilation or testing, whether that's via human eyes or improve
> machine readability.
> 
> We also need to run our new "ci/check-unignored-build-artifacts.sh" on
> success() in the CI now, just like we already had a step conditional
> on failure() running ci/print-test-failures.sh.
> 
> The reason we used a "ci/run-build-and-tests.sh" wrapper in the first
> place had to do with Travis CI-specific constraints that no longer
> apply to us, as the Travis CI support has been removed.
> 
> Instead we can configure the CI in an earlier step by running
> "ci/lib.sh", which under GitHub CI will write the environment
> variables we need to the "$GITHUB_ENV" file.
> 
> We'll then have access to them in subsequent steps, and crucially
> those variables will be prominently visible at the start of each step
> via an expandable drop-down in the UI.drop-do.
> 
> I.e. this changes the CI run from a top-down flow like (pseudocode):
> 
>   - job:
>     - step1:
>       - use ci/lib.sh to set env vars
>       - run a script like ci/run-build-and-tests.sh
>     - step2:
>       - if: failure()
>       - use ci/lib.sh to set env vars
>       - run ci/print-test-failures.sh
> 
> To:
> 
>   - job:
>     - step1:
>       - set variables in $GITHUB_ENV using ci/lib.sh
>     - step2:
>       - make
>     - step3:
>       - make test
>     - step4:
>       - if: failure()
>       - run ci/print-test-failures.sh
>     - step5:
>       - if: success()
>       - run ci/check-unignored-build-artifacts.sh
> 
> There is a proposal[2] to get some of the benefits of this approach by
> not re-arranging our variable setup in this way, but to instead use
> the GitHub CI grouping syntax to focus on the relevant parts of "make"
> or "make test" when we have failures.
> 
> Doing it this way makes for better looking GitHub CI UI, and lays much
> better ground work for our CI going forward. Because:
> 
>   * The CI logic will be more portable to a future CI system, since a
>     common feature of them is to run various commands in sequence, but
>     a future system won't necessarily support the GitHub-specifics
>     syntax of "grouping" output within a "step".
> 
>     Even if those systems don't support a "$GITHUB_ENV" emulating will
>     be much easier than to deal with some CI-specific grouping syntax.
> 
>   * At the start of every step the GitHub CI presents an expandable
>     list of environment variables from "$GITHUB_ENV". We'll now see
>     exactly what variables affected that step (although we currently
>     overshoot that a bit, and always define all variables).
> 
>   * CI failures will be easier to reproduce locally, as this makes the
>     relevant ci/* scripts something that sets up our environment, but
>     leaves "make" and "make test" working as they do locally.
> 
>     To reproduce a run the user only needs to set the variables
>     discussed in the drop-down above, either manually or by running
>     "ci/lib.sh".
> 
>   * The output will be less verbose. The "ci/lib.sh" script uses "set
>     -x", and before this e.g. "ci/static-analysis.sh" would start with
>     40 lines of trace output, culminating in using "export" to export
>     the relevant environment variables.
> 
>     Now that verbosity is in the earlier "ci/lib.sh" step, and not in
>     any subsequent one. The "make" targets then start out with the
>     relevant output non-trace output right away.

s/output non-trace output/non-trace output/

>   * If we do want to use the grouping syntax within a "step" it'll now
>     be easier to do so. It doesn't support nesting, so we'd have to
>     make a choice between using it for e.g. "make" v.s. "make test", or
>     individual test failures. See "sadly" in [3].
> 
> 1. https://lore.kernel.org/git/211120.86k0h30zuw.gmgdl@evledraar.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/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>