You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/02/16 11:19:55 UTC
[GitHub] [maven-surefire] NilsRenaud opened a new pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
NilsRenaud opened a new pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471
This PR fixes : https://issues.apache.org/jira/browse/SUREFIRE-1909
We decided to go with the drop in replacement of `--add-exports` with `--add-opens`, as suggested in the issue.
To add to the background of this issue, a previous PR has been created, introducing the option to use either `add-exports` or `add-opens`, the PR is still accessible here : https://github.com/apache/maven-surefire/pull/461
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r807914433
##########
File path: surefire-its/src/test/resources/junit5-modulepath/src/main/java/com/app/Main.java
##########
@@ -0,0 +1,34 @@
+package com.app;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.joda.time.DateTime;
+
+public class Main
+{
+ public static void main( String... args )
+ {
+ System.out.println( "module path => " + System.getProperty( "jdk.module.path" ) );
Review comment:
@NilsRenaud
So, these lines are used as a debug info?
You can maybe utilize it and verify the classpath in one line, and modulapath in the second line via combining Java Hamcrest matchers.
Example `validator.assertThatLogLine( containsString( "Running jiras.surefire1082.Jira1082Test" ), is( 1 ) );`
Example with matching one line: `allOf( containsString( "jar1" ), containsString( "jar2" ), ... )`
The second argument `is(1)` only means the number of lines matching this criteria and it is 1 in this IT.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#issuecomment-1041493137
Let's keep this surivar open during several release versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r808042194
##########
File path: surefire-its/src/test/resources/junit5-modulepath/src/main/java/com/app/Main.java
##########
@@ -0,0 +1,34 @@
+package com.app;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.joda.time.DateTime;
+
+public class Main
+{
+ public static void main( String... args )
+ {
+ System.out.println( "module path => " + System.getProperty( "jdk.module.path" ) );
Review comment:
After you have deleted it, we can force the build to run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r808089493
##########
File path: surefire-its/src/test/resources/junit5-modulepath/pom.xml
##########
@@ -0,0 +1,46 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>foo</groupId>
+ <artifactId>app</artifactId>
+ <version>1.0.0-SNAPSHOT</version>
+
+ <name>app</name>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.release>${java.specification.version}</maven.compiler.release>
+ </properties>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
Review comment:
It's better to use the [latest version](https://maven.apache.org/plugins/maven-compiler-plugin/) because we fixed some bugs and it would be nice to come over those in Surefire too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] NilsRenaud commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
NilsRenaud commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r807824894
##########
File path: surefire-its/src/test/resources/junit5-modulepath/src/test/java/com/app/AppTest.java
##########
@@ -0,0 +1,37 @@
+package com.app;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.junit.jupiter.api.Test;
+
+public class AppTest
+{
+ @Test
+ void testNoop()
Review comment:
This is the point of the test : to run package private test methods as suggested in [JUnit 5 user guide](https://junit.org/junit5/docs/current/user-guide/#writing-tests)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] NilsRenaud commented on pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
NilsRenaud commented on pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#issuecomment-1042693410
Thanks @Tibor17 for your mentoring all along the way, that was great !
Since I'm around, I've 2 questions :
- have you any insight on the 3.0 final release date ?
- And I guess the answer will be no, but would it be possible to backport this fix on 2.22 release ? ^^'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] NilsRenaud commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
NilsRenaud commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r808025692
##########
File path: surefire-its/src/test/resources/junit5-modulepath/src/main/java/com/app/Main.java
##########
@@ -0,0 +1,34 @@
+package com.app;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.joda.time.DateTime;
+
+public class Main
+{
+ public static void main( String... args )
+ {
+ System.out.println( "module path => " + System.getProperty( "jdk.module.path" ) );
Review comment:
I shamelessly copied an existing test with this content.
They are not useful because the `java` arg file content is displayed anyway. And I'm not really sure what I should verify with these paths.
So I rather like to delete these debug lines. WDYT ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 merged pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 merged pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] NilsRenaud commented on pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
NilsRenaud commented on pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#issuecomment-1042945234
Backport PR created targeting 2.22.3 : https://github.com/apache/maven-surefire/pull/473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r807914433
##########
File path: surefire-its/src/test/resources/junit5-modulepath/src/main/java/com/app/Main.java
##########
@@ -0,0 +1,34 @@
+package com.app;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.joda.time.DateTime;
+
+public class Main
+{
+ public static void main( String... args )
+ {
+ System.out.println( "module path => " + System.getProperty( "jdk.module.path" ) );
Review comment:
@NilsRenaud
So, these lines are used as a debug info?
You can maybe utilize it and verify the classpath in one line, and modulapath in the second line via combining Java Hamcrest matchers.
Example:
`validator.assertThatLogLine( containsString( "Running jiras.surefire1082.Jira1082Test" ), is( 1 ) );`
Example with matching one line: `allOf( containsString( "jar1" ), containsString( "jar2" ), ... )`
The second argument `is(1)` only means the number of lines matching this criteria and it is 1 in this IT.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#issuecomment-1042718932
@NilsRenaud
I have backported many fixes to the branch [release/2.22.3](https://github.com/apache/maven-surefire/tree/release/2.22.3). Truly I wanted to cut the release `2.22.3` but hopefully I have not done it because a critical issue was fixed.
Feel free to Cherry-Pick your commit from master to the target branch and I will confirm that on GH.
Btw, I am facing one more issue where the JVM hangs, I know the root cause, and it will be pushed right after yours.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#discussion_r807914433
##########
File path: surefire-its/src/test/resources/junit5-modulepath/src/main/java/com/app/Main.java
##########
@@ -0,0 +1,34 @@
+package com.app;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.joda.time.DateTime;
+
+public class Main
+{
+ public static void main( String... args )
+ {
+ System.out.println( "module path => " + System.getProperty( "jdk.module.path" ) );
Review comment:
So, these lines are used as a debug info?
You can maybe verify the classpath in one line, and modulapath in the second line via combining Java Hamcrest matchers.
Example `validator.assertThatLogLine( containsString( "Running jiras.surefire1082.Jira1082Test" ), is( 1 ) );`
Example with matching one line: `allOf( containsString( "jar1" ), containsString( "jar2" ), ... )`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 removed a comment on pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 removed a comment on pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#issuecomment-1041493137
Let's keep this surivar open during several release versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #471: [SUREFIRE-1909] - Support JUnit 5 reflection access by changing add-exports to add-opens
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #471:
URL: https://github.com/apache/maven-surefire/pull/471#issuecomment-1041830759
@NilsRenaud
The build is running...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org