[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#26803: [PATCH 16/36] gnu: Add java-commons-lang.
From: |
Roel Janssen |
Subject: |
bug#26803: [PATCH 16/36] gnu: Add java-commons-lang. |
Date: |
Wed, 10 May 2017 16:23:14 +0200 |
User-agent: |
mu4e 0.9.18; emacs 25.2.1 |
Ricardo Wurmus writes:
> Roel Janssen <address@hidden> writes:
>
>> Ricardo Wurmus writes:
>>
>>> From: Hartmut Goebel <address@hidden>
>>>
>>> * gnu/packages/java.scm (java-commons-lang): New variable.
>>>
>>> Co-authored-by: Ricardo Wurmus <address@hidden>
>>> ---
> […]
>>> + (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.
I agree. So, LGTM!
Kind regards,
Roel Janssen
bug#26803: [PATCH 16/36] gnu: Add java-commons-lang., Ricardo Wurmus, 2017/05/06
bug#26803: [PATCH 18/36] gnu: Add java-commons-cli., Ricardo Wurmus, 2017/05/06
bug#26803: [PATCH 17/36] gnu: Add java-commons-lang3., Ricardo Wurmus, 2017/05/06
bug#26803: [PATCH 22/36] gnu: java-hamcrest-core: Declare test target., Ricardo Wurmus, 2017/05/06
bug#26803: [PATCH 23/36] gnu: Add java-hamcrest-all., Ricardo Wurmus, 2017/05/06