GNU bug report logs - #59487
[PATCH 1/2] build-system/dune: Automatically deduce test-target in most cases.

Previous Next

Package: guix-patches;

Reported by: raingloom <raingloom <at> riseup.net>

Date: Tue, 22 Nov 2022 19:48:02 UTC

Severity: normal

Tags: patch

Done: Julien Lepiller <julien <at> lepiller.eu>

Bug is archived. No further changes may be made.

Full log


Message #29 received at 59487 <at> debbugs.gnu.org (full text, mbox):

From: pukkamustard <pukkamustard <at> posteo.net>
To: Csepp <raingloom <at> riseup.net>
Cc: julien <at> lepiller.eu, 59487 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 1/2] build-system/dune: Automatically deduce
 test-target in most cases.
Date: Thu, 12 Jan 2023 15:42:26 +0000
Thanks for this! I think it is a valuable improvement.

Csepp <raingloom <at> riseup.net> writes:

> From: raingloom <raingloom <at> riseup.net>
>
> guix/build-system/dune.scm (dune-build): tests? defaults to #f.

This should be: "test-target defaults to #f".

> +    (let ((program (if jbuild? "jbuilder" "dune"))
> +          (test-target (or test-target
> +                           (cond
> +                            ((file-exists? "tests") "tests")
> +                            ((file-exists? "test") "test")
> +                            (else ".")))))
> +      (apply invoke program "runtest"
> +             (append (if test-target (list test-target) '())
> +                     (if package (list "-p" package)
>                           dune-release-flags)
>                       test-flags))))
>    #t)

I think what Julien ment (and I agree) is that you can completely drop
the checks for the files/directories "tests" or "test" to exist.

In your patch, if test-target is #f and "test" or "tests" do not exist,
then the we will run:

`dune runtest -p package .`

but we could (and maybe should) run just:

`dune runtest -p package`

In fact, we should run this even if the "test" or "tests" directories
exist (otherwise we might miss running some tests placed in other
directories).

So this would be enough:

> +    (let ((program (if jbuild? "jbuilder" "dune")))
> +      (apply invoke program "runtest"
> +             (append (if test-target (list test-target) '())
> +                     (if package (list "-p" package)
>                           dune-release-flags)
>                       test-flags)))

Thinking of this, maybe we can drop the `test-target` argument
completely. wdyt?

-pukkamustard




This bug report was last modified 2 years and 118 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.