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

[Csepp <raingloom@riseup.net>]

Csepp <raingloom@riseup.net> writes:

> pukkamustard <pukkamustard@posteo.net> writes:
>
>> 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
>
> Hmm, makes sense.  I'll test what complete removal does.

Well ocaml-ppxlib fails and I should be working my MirageOS thesis
stuff, so I'll leave this up to you.  Or I'll get back to it once I
figure out how cross-toolchains are supposed to work.