diff mbox

[2/3] guix: ant-build-system: add empty `tests` target to default build.xml.

Message ID 1473067474-7485-3-git-send-email-h.goebel@crazy-compilers.com
State New
Headers show

Commit Message

Hartmut Goebel Sept. 5, 2016, 9:24 a.m. UTC
This avoids the need to set #:tests? #f whenever using #:jar-name
(and thus using the default build.xml).

* guix/build/ant-build-system.scm (default-build.xml): Add attribute
  to sxml expression.
---
 guix/build/ant-build-system.scm | 1 +
 1 file changed, 1 insertion(+)

Comments

Vincent Legoll Sept. 5, 2016, 10:59 a.m. UTC | #1
Hello,

On Mon, Sep 5, 2016 at 11:24 AM, Hartmut Goebel
<h.goebel@crazy-compilers.com> wrote:
> This avoids the need to set #:tests? #f whenever using #:jar-name
> (and thus using the default build.xml).

Isn't it a bit misleading to have a test target that does no real
testing, or am I
misunderstanding this patch ?
Hartmut Goebel Sept. 5, 2016, 11:32 a.m. UTC | #2
Hi,

Am 05.09.2016 um 12:59 schrieb Vincent Legoll:
> On Mon, Sep 5, 2016 at 11:24 AM, Hartmut Goebel
> <h.goebel@crazy-compilers.com> wrote:
>> This avoids the need to set #:tests? #f whenever using #:jar-name
>> (and thus using the default build.xml).
> Isn't it a bit misleading to have a test target that does no real
> testing, or am I
> misunderstanding this patch ?

When specifying #:jar-file, the ant-builder will create a build.xml.
This build.xml did not include a test-target at all. So one *always* had
to include "#:tests? #f" to disable calling the (non-existing)
test-target - otherwise build would fail. But adding "#:tests? #f" is
just redundant.
Danny Milosavljevic Sept. 5, 2016, 8:26 p.m. UTC | #3
Hi Hartmut,

On Mon, 5 Sep 2016 13:32:44 +0200
Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:

> When specifying #:jar-file, the ant-builder will create a build.xml.
> This build.xml did not include a test-target at all. So one *always* had
> to include "#:tests? #f" to disable calling the (non-existing)
> test-target - otherwise build would fail. But adding "#:tests? #f" is
> just redundant.

I see what you mean. But is it still possible to easily find out which packages have tests and which haven't after your patch?

I think being able to know that is important since it's prudent to trust a package without tests (much) less.

Also, would it be possible to auto-discover jUnit tests instead?
Vincent Legoll Sept. 5, 2016, 8:57 p.m. UTC | #4
Hello

On Mon, Sep 5, 2016 at 10:26 PM, Danny Milosavljevic
<dannym@scratchpost.org> wrote:
> I think being able to know that is important since it's prudent to trust a package without tests (much) less.

That's my feeling too
Hartmut Goebel Sept. 6, 2016, 7:13 a.m. UTC | #5
Am 05.09.2016 um 22:26 schrieb Danny Milosavljevic:
> I see what you mean. But is it still possible to easily find out which packages have tests and which haven't after your patch?

We are talking about the case when #:jar-name is specified. Only in this
case a build.xml will be generated.

Situation prior to this patch:

The generated build.xml does not include a "test" target. Phase "check"
will fail, since it wants to build the (non-existing) "test" target.
Solutions are a) set #:test? #f, b) remove phase "check" or c) define
your own "check" phase. If you want to run tests on the package, you
need to replace the "check" phase by something meaningful.

Situation avert the patch:

The generated build.xml include a "test" target. Phase "check" will
pass, since there is now a dummy "test" target. If you want to run tests
on the package, you need to replace the "check" phase by something
meaningful.


So the *behaviour* did not check. The only thing that has changes is the
need to either a) set #:test? #f, b) remove phase "check".

As of today, only four packages define a #:jar-name and all of these had
':tests? #f'.

> Also, would it be possible to auto-discover jUnit tests instead?

Maybe. But this is beyond my Java knowledge.
Vincent Legoll Sept. 6, 2016, 10:52 a.m. UTC | #6
Hello,

What about:

if jar-file makes you generate a build.xml file, also remove the check phase...
diff mbox

Patch

diff --git a/guix/build/ant-build-system.scm b/guix/build/ant-build-system.scm
index fe7bae5..2cc6bb9 100644
--- a/guix/build/ant-build-system.scm
+++ b/guix/build/ant-build-system.scm
@@ -70,6 +70,7 @@ 
                                (arg (@ (line ,(string-append "-cf ${jar.dir}/" jar-name
                                                              " -C ${classes.dir} ."))))))
 
+                 (target (@ (name "tests")))
                  (target (@ (name "install"))
                          (copy (@ (todir "${dist.dir}"))
                                (fileset (@ (dir "${jar.dir}"))