Re: [PATCH v2] CI: don't explicitly pick "bash" shell outside of Windows, fix regression

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ævar

On 07/12/2022 12:03, Ævar Arnfjörð Bjarmason wrote:
> When the "js/ci-github-workflow-markup" topic was originally merged in
> [1] it included a change to get rid of the "ci/print-test-failures.sh"
> step[2]. This was then brought back in [3] as part of a fix-up patches
> on top[4].
> 
> The problem was that [3] was not a revert of the relevant parts of
> [2], but rather copy/pasted the "ci/print-test-failures.sh" step that
> was present for the Windows job to all "ci/print-test-failures.sh"
> steps. The Windows steps specified "shell: bash", but the non-Windows
> ones did not.
> 
> This broke the "ci/print/test-failures.sh" step for the "linux-musl"
> job, where we don't have a "bash" shell, just a "/bin/sh" (a
> "dash"). This breakage was reported at the time[5], but hadn't been
> fixed.
> 
> It would be sufficient to change this only for "linux-musl", but let's
> change this for both "regular" and "dockerized" to omit the "shell"
> line entirely, as we did before [2].
> 
> Let's also change undo the "name" change that [3] made while
> copy/pasting the "print test failures" step for the Windows job. These
> steps are now the same as they were before [2], except that the "if"
> includes the "env.FAILED_TEST_ARTIFACTS" test.

What's the motivation for this part of the change (which is completely 
unrelated to the choice of shell)? Having the test failures under "Print 
test failures" makes it easy for new contributors to see where to click 
to see the full output for test failures. Now they will appear under 
"Run ci/print-test-failures.sh" which while not terrible is not as clear.

Best Wishes

Phillip

> 1. fc5a070f591 (Merge branch 'js/ci-github-workflow-markup', 2022-06-07)
> 2. 08dccc8fc1f (ci: make it easier to find failed tests' logs in the
>     GitHub workflow, 2022-05-21)
> 3. 5aeb145780f (ci(github): bring back the 'print test failures' step,
>     2022-06-08)
> 4. d0d96b8280f (Merge branch 'js/ci-github-workflow-markup', 2022-06-17)
> 5. https://lore.kernel.org/git/220725.86sfmpneqp.gmgdl@evledraar.gmail.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 
> I forgot the Signed-off-by in v1, sorry.
> 
> Range-diff against v1:
> 1:  08dc682926a ! 1:  c34fd06623a CI: don't explicitly pick "bash" shell outside of Windows, fix regression
>      @@ Commit message
>           4. d0d96b8280f (Merge branch 'js/ci-github-workflow-markup', 2022-06-17)
>           5. https://lore.kernel.org/git/220725.86sfmpneqp.gmgdl@evledraar.gmail.com/
>       
>      +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>      +
>        ## .github/workflows/main.yml ##
>       @@ .github/workflows/main.yml: jobs:
>            - uses: actions/checkout@v2
> 
>   .github/workflows/main.yml | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 9afacfa0b33..1a86f6a8ce1 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -265,10 +265,8 @@ jobs:
>       - uses: actions/checkout@v2
>       - run: ci/install-dependencies.sh
>       - run: ci/run-build-and-tests.sh
> -    - name: print test failures
> +    - run: ci/print-test-failures.sh
>         if: failure() && env.FAILED_TEST_ARTIFACTS != ''
> -      shell: bash
> -      run: ci/print-test-failures.sh
>       - name: Upload failed tests' directories
>         if: failure() && env.FAILED_TEST_ARTIFACTS != ''
>         uses: actions/upload-artifact@v2
> @@ -297,10 +295,8 @@ jobs:
>       - uses: actions/checkout@v1
>       - run: ci/install-docker-dependencies.sh
>       - run: ci/run-build-and-tests.sh
> -    - name: print test failures
> +    - run: ci/print-test-failures.sh
>         if: failure() && env.FAILED_TEST_ARTIFACTS != ''
> -      shell: bash
> -      run: ci/print-test-failures.sh
>       - name: Upload failed tests' directories
>         if: failure() && env.FAILED_TEST_ARTIFACTS != ''
>         uses: actions/upload-artifact@v1