[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tests/examples: Add running of workflow examples
From: |
Ricardo Wurmus |
Subject: |
Re: [PATCH] tests/examples: Add running of workflow examples |
Date: |
Fri, 29 Apr 2022 22:32:08 +0200 |
User-agent: |
mu4e 1.6.10; emacs 28.0.50 |
Hi Olivier,
thanks for the patch!
> End to end testing of pre-defined scenarios are a good way to check for
> regression.
>
> Here we introduce testing of some examples available in the documentation.
> That
> way, we're sure that new users should be able to run them without problems.
>
> Each scenario is a different test and is run in a different temporary
> directory which get destroyed if the scenario succeeded.
It’s a good idea to run the examples. However, this requires a fully
functional Guix installation (which we don’t have when building with
Guix, for example), so we should not include them in “make check” but
add a separate target for these tests. An alternative may be to do what
Guix does for system tests, but I’d be okay with just having a separate
target that is run manually before releases or in CI.
> +(define-syntax-rule (with-directory-excursion dir body body* ...)
> + (let ((old-dir (getcwd)))
> + (dynamic-wind
> + (lambda () (chdir dir))
> + (lambda () body body* ...)
> + (lambda () (chdir old-dir)))))
> +
> +(with-directory-excursion
> + (string-append top-srcdir "/doc/examples")
> +
> + (define (process-success? status)
> + (= 0 (or (status:exit-val status)
> + (status:term-sig status))))
Use ZERO? here.
> + (define scenarios
> + (list "extended-example-workflow.w"))
Should these better be discovered automatically via SCANDIR?
> + (for-each (lambda (example)
> + (test-assert example
> + (let* ((tmp-dir (mkdtemp
> + (format #f "gwl-example-~a.XXXXXX"
> example)))
> + (abs-example (canonicalize-path example))
> + (success?
> + (with-directory-excursion tmp-dir
> + (process-success?
> + (system
> + (format #f "guix workflow run -fc ~a -l all"
> + abs-example))))))
Please don’t use SYSTEM. How about
(system* "guix" "workflow" "run" "--force" "--container"
"--log-events=all" (canonicalize-path example))
?
> + (if success?
> + (system* "rm" "-rf" tmp-dir)
Why shell out to RM when we have DELETE-FILE and its recursive friend in
Guix? I’d also rather move clean-up work to a DYNAMIC-UNWIND handler.
> + (format (error-output-port)
> + "Example directory: ~a\n" tmp-dir))
Nitpick: ~% instead of \n.
> + success?)))
> + scenarios))
This FOR-EACH loop combines test definition with test running, which
seems wrong to me. Maybe SRFI-64 is not the best fit for tests that
only care about whether a shell command was run successfully. Perhaps
we should do as Guix does and just have a shell script to run these
tests.
--
Ricardo