Subject: bug#26803: [PATCH 16/36] gnu: Add java-commons-lang.

From: Ricardo Wurmus <[email-masked]>
Date: Wed, 10 May 2017 16:13:44 +0200

Roel Janssen <[email-masked]> writes:

> Ricardo Wurmus writes:
>
>> From: Hartmut Goebel <[email-masked]>
>>
>> * gnu/packages/java.scm (java-commons-lang): New variable.
>>
>> Co-authored-by: Ricardo Wurmus <[email-masked]>
>> ---
[…]
>> +    (arguments
>> +     `(#:test-target "test"
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'build 'build-javadoc ant-build-javadoc)
>> +         (add-before 'check 'disable-failing-test
>> +           (lambda _
>> +             ;; Disable a failing test
>> +             (substitute* "src/test/java/org/apache/commons/lang/\
>> +time/FastDateFormatTest.java"
>> +               (("public void testFormat\\(\\)")
>> +                "public void disabled_testFormat()"))
>> +             #t))
>
> Since you're renaming the function, I suppose removing the function
> would also work, which would not create any "dead code".  But that
> probably requires a separate patch file, which will break more easily
> on an update.
>
> So long story short:  This is OK to me, even though I don't like
> producing code that won't be run anyway.

I agree in principle, but this is the Java way of disabling a test.
Tests are considered tests only when the procedures start with the
string “test”, so renaming the test causes it not to be executed.
Removing requires much more effort because it requires an actual patch.
Since it’s just a test (and not library code) I think it’s acceptable
not to delete it.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net