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

[pukkamustard]

Thanks for this! I think it is a valuable improvement.

Csepp <raingloom@riseup.net> writes:

> From: raingloom <raingloom@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