Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move TCKs to Maven and JUnit5 #36 #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Oct 12, 2021

#36

Some comments for reviewers:

  • Sigtest needs to be regenerated with a version that is understood by org.netbeans.tools:sigtest-maven-plugin:1.4. I don't know how to do that, maybe somebody can help here. This is the only test that is failing.
  • It is done in similar way than jsonp tcks. There are 2 root maven modules, one contains the TCKs that can be used by different implementations, and other module is executing it with angus-activation implementation.
  • Test UnnamedModule_Test was problematic. You will probably find tricky the maven-surefire-plugin configuration, but I didn't come up with an easier solution.
  • I removed docker files because I think this is not needed because there are no dependencies on external resources.
  • I didn't update the Jenkins file. This is something pending to be done.
  • Pluggability folders does not contain anything useful yet. That is prepared for this other issue Add tests for SPI added for JAF 2.1 #31

<forkCount>1</forkCount>
<reuseForks>false</reuseForks>
<!-- Required for test javasoft.sqe.tests.api.jakarta.activation.Module.UnnamedModule_Test -->
<argLine>--add-modules ALL-MODULE-PATH --module-path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you experiment with <useModulePath>true/false</useModulePath>? Did you try M4 version of surefire?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried now with M4 but I was not able to make it work.

I also tried with useModulePath that is true by default. I have run maven with -X to see the arguments and I don't see any --module-path. In the description it says:

Enabled by default. If enabled, module-info.java exists and 
 executes with JDK 9+, modular path is used.

Note that module-info.java in the test does not exist. We need to create also a module-info.java in this module. But, if we do this, we cannot test that unnamed modules are able to run jakarta.activation.

In my opinion surefire-plugin needs more customization.classpathDependencyExcludes is good, but we also need moduleDependencyIncludes. With that we could exclude jakarta.activation from classpath and add it in modulepath

@jbescos jbescos force-pushed the issue36 branch 2 times, most recently from baf66e0 to e714c11 Compare October 13, 2021 06:53
@jbescos
Copy link
Member Author

jbescos commented Oct 13, 2021

I have modified UnnamedModule_Test to skip if modules are not enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants