[ 
https://issues.apache.org/jira/browse/LUCENE-8146?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16348273#comment-16348273
 ] 

Uwe Schindler edited comment on LUCENE-8146 at 2/1/18 9:28 AM:
---------------------------------------------------------------

Hi,
the Surefire bug (I call it a bug, because there is no way to explicitely pass 
a non-existent property, the whole ANT build infrstratcture depends on sysprops 
being set or not as true/false). There are other system properties in Lucene's 
test that should be non-existent if undefined!!!

Maven surefire should fix their bugs and allow to unset a property if it is 
empty. Just switching from one way to another way is horrible. It's only Maven 
that is affected by this, Ant works fine and Gradle, too. With this change it 
is for example impossible to execute Lucene's tests without using a security 
manager, because you cannot simply unset the security manager property (the ANT 
build can do this). In Maven it would pass an empty string and that would 
enable the security manager with default implementation, breaking everything!

About your patch: You can still move the code to a common location, but please, 
please don't create new public classes because of that! Just add it as a method 
to TestUtils or another utility class - done. There is no need to add a 
separate class. But as we have to modify Lucene's core code here (not test 
code) I tend to stay without refactoring out new public methods, visible to 
everyone, so lets copypaste the easy fix.


was (Author: thetaphi):
Hi,
the Surefire bug (I call it a bug, because there is no way to explicitely pass 
a non-existent property, the whole ANT build infrstratcture depends on sysprops 
being set or not as true/false). There are other system properties in Lucene's 
test that should be non-existent if undefined!!!

Maven surefire should fix their bugs and allow to unset a property if it is 
empty. Just switching from one way to another way is horrible. It's only Maven 
that is affected by this, Ant works fine and Gradle, too. With this change it 
is for example impossible to execute Lucene's tests without using a security 
manager, because you cannot simply unset the security manager property (the ANT 
build can do this). In Maven it would pass an empty string and that would 
enable the security manager with default implementation, breaking everything!

About your patch: You can still move the code to a common location, but please, 
please don't create new public classes because of that! Just add it as a method 
to TestUtils - done. There is no need to add a separate class.

> Unit tests using StringHelper fail with ExceptionInInitializerError for maven 
> surefire >= 2.18
> ----------------------------------------------------------------------------------------------
>
>                 Key: LUCENE-8146
>                 URL: https://issues.apache.org/jira/browse/LUCENE-8146
>             Project: Lucene - Core
>          Issue Type: Bug
>    Affects Versions: 7.2.1
>            Reporter: Julien Massenet
>            Priority: Minor
>         Attachments: LUCENE-8146-seed_issue.tar.gz, LUCENE-8146_v1.patch, 
> LUCENE-8146_v2.patch
>
>
> This happens when multiple conditions are met:
>  * The client code is built with Maven
>  * To execute its unit tests, the client code relies on the 
> {{maven-surefire-plugin}}, with a version greater than 2.17 (last working 
> version)
>  * The client code uses the {{org.apache.lucene.util.StringHelper}} class 
> (even transitively)
>  * The client is configured as with the standard Lucene maven build (i.e. it 
> is possible to fix the test seed using the {{tests.seed}} property)
> There was a change in Surefire's behavior starting with 2.18: when a property 
> is empty, instead of not sending it to the test runner, it will be sent with 
> an empty value.
> This behavior can be observed with the attached sample project:
>  * {{mvn test}}: fails with a {{java.lang.ExceptionInInitializerError}}
>  * {{mvn test -Dtests.seed=123456}}: succeeds because the property is set to 
> a real value
>  * {{mvn test -Dsurefire.version=2.17}}: succeeds because the surefire 
> version is lower than 2.18
> Attached is a patch (built against \{{branch_7x}}) that centralizes accesses 
> to the {{tests.seed}} system property; it also makes sure that if it is 
> empty, it is treated as absent.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to