From: Ricardo Wurmus <rekado@elephly.net>
To: Olivier Dion <olivier.dion@polymtl.ca>
Cc: gwl-devel@gnu.org
Subject: Re: [PATCH] tests/examples: Add running of workflow examples
Date: Sat, 07 May 2022 08:46:31 +0200 [thread overview]
Message-ID: <871qx5u99a.fsf@elephly.net> (raw)
In-Reply-To: <87sfpva9ch.fsf@laura>
Olivier Dion <olivier.dion@polymtl.ca> writes:
>>> + (define scenarios
>>> + (list "extended-example-workflow.w"))
>>
>> Should these better be discovered automatically via SCANDIR?
>
> Well some example are not full workflow. Only processes. So I think that
> yes, if we set a common prefix for full workflow:
>
> (scandir "." (cut string-prefix "exemple-workflow" <>))
>
> However, it's nice -- even more true when debugging -- to be able to
> cherry-pick the tests. This is easier with the list above by commenting
> tests that are passing.
That makes sense.
>> 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.
>
> I hesitated to include Guix's module in test. If you're okay with it, I
> will use the helpers available in Guix. I concur that the cleanup
> should be in dynamic-unwind.
Yes, using the helpers from Guix is fine. The GWL doesn’t make much
sense without Guix anyway :)
>>> + (format (error-output-port)
>>> + "Example directory: ~a\n" tmp-dir))
>>
>> Nitpick: ~% instead of \n.
>
> Is there a reason why? I don't mind I just never really understood why
> scheme has this special format rule for newline.
I don’t know if there’s a reason for it, but I prefer it for consistency.
>>> + 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.
>
> What do you find wrong about it? We could re-write it as:
>
> (define (run-example path)
> ...)
>
> (test-assert (run-example "extended-workflow.w"))
> (test-assert (run-example "..."))
>
> if you like it better. However, we lose the power of SCANDIR mentioned
> above.
Hmm, you’re right about that. If you could split this up into smaller
procedures (one definition of RUN-EXAMPLE and another that loops
TEST-ASSERT and RUN-EXAMPLE on the test files) I’d already be happy :)
> Let me know, I will send a v2 with your above recommendations and answers
> to my questions.
Thank you!
--
Ricardo
next prev parent reply other threads:[~2022-05-07 6:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-29 17:55 [PATCH] tests/examples: Add running of workflow examples Olivier Dion
2022-04-29 20:32 ` Ricardo Wurmus
2022-04-29 21:05 ` Olivier Dion via
2022-05-07 6:46 ` Ricardo Wurmus [this message]
2022-05-07 16:47 ` [PATCH v2] " Olivier Dion
2022-06-03 9:06 ` Ricardo Wurmus
2022-06-03 13:11 ` Olivier Dion via
2022-06-03 15:46 ` Ricardo Wurmus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871qx5u99a.fsf@elephly.net \
--to=rekado@elephly.net \
--cc=gwl-devel@gnu.org \
--cc=olivier.dion@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.