Re: [PATCH 4/5] ci: split out logic to set up failed test artifacts

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2191 bytes --]

On Fri, Nov 03, 2023 at 11:35:01PM +0100, Christian Couder wrote:
> On Thu, Oct 26, 2023 at 10:01 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > We have some logic in place to create a directory with the output from
> > failed tests, which will then subsequently be uploaded as CI artifact.
> > We're about to add support for GitLab CI, which will want to reuse the
> > logic.
> >
> > Split the logic into a separate function so that it is reusable.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  ci/lib.sh | 40 ++++++++++++++++++++++------------------
> >  1 file changed, 22 insertions(+), 18 deletions(-)
> >
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index 957fd152d9c..33005854520 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -137,6 +137,27 @@ handle_failed_tests () {
> >         return 1
> >  }
> >
> > +create_failed_test_artifacts () {
> > +       mkdir -p t/failed-test-artifacts
> > +
> > +       for test_exit in t/test-results/*.exit
> > +       do
> > +               test 0 != "$(cat "$test_exit")" || continue
> > +
> > +               test_name="${test_exit%.exit}"
> > +               test_name="${test_name##*/}"
> > +               printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
> > +               echo "The full logs are in the 'print test failures' step below."
> > +               echo "See also the 'failed-tests-*' artifacts attached to this run."
> > +               cat "t/test-results/$test_name.markup"
> > +
> > +               trash_dir="t/trash directory.$test_name"
> > +               cp "t/test-results/$test_name.out" t/failed-test-artifacts/
> > +               tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
> > +       done
> > +       return 1
> 
> Nit: I think it's more the responsibility of handle_failed_tests() to
> `return 1` than of create_failed_test_artifacts(). I understand that
> putting it into this new function means one more line that is common
> to several CI tools though.
> 
> Otherwise everything in this series LGTM. Thanks!

Good point indeed. Not sure whether this is worth a reroll though?

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]