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

System-Rules based tests migration #1932

Closed
remkop opened this issue Jan 24, 2023 · 0 comments
Closed

System-Rules based tests migration #1932

remkop opened this issue Jan 24, 2023 · 0 comments
Labels
theme: build An issue or change related to the build system
Milestone

Comments

@remkop
Copy link
Owner

remkop commented Jan 24, 2023

Related to #1930:

Problem

DefaultProviderTest and HelpAnsiTest tests using JUnit 4 and Stephan Birkner’s System-Rules are broken on Java 18-ea (UnsupportedOperationException for setSecurityManager).

I also saw some InaccessibleObjectException errors in the CI/CD build logs on Java 16+:

picocli.HelpAnsiTest > testTextWithMultipleStyledSections FAILED
    java.lang.reflect.InaccessibleObjectException at AccessibleObject.java:354

Analysis

The UnsupportedOperationException in Java 18-ea is due to the EnvironmentVariables rule reflection no longer working due to removal of the System::setSecurityManager API in Java 18-ea. However, it looks like this API was not actually removed in Java 18's GA release.

It is not clear what caused the InaccessibleObjectException.
All tests seem to pass on Zulu Java 18 (with a warning):

WARNING: A terminally deprecated method in java.lang.System has been called

> Task :test
WARNING: System::setSecurityManager has been called by
     org.junit.contrib.java.lang.system.ProvideSecurityManager 
    (file:/home/runner/.gradle/caches/modules-2/files-2.1/com.github.stefanbirkner/
    system-rules/1.19.0/d541c9a1cff0dda32e2436c74562e2e4aa6c[88]  
    (https://github.com/remkop/picocli/actions/runs/3993249999/jobs/6849880365#step:9:89)cd/system-rules-1.19.0.jar)
WARNING: Please consider reporting this to the maintainers of 
    org.junit.contrib.java.lang.system.ProvideSecurityManager
WARNING: System::setSecurityManager will be removed in a future release

At this moment we are unable to test with Java 19 because Gradle 7.4 does not support it.
This problem will resurface when the System::setSecurityManager API is actually removed.

Solution

The solution is to move all tests that use the System-Rules EnvironmentVariables rule to the picocli-legacy-tests module.

Also, add a module picocli-tests-junit5 that uses JUnit 5 and System Lambda instead. Migrate Environment and System exit tests to this module.

This module can also host other tests that require Java 8 that are currently in the picocli-annotation-processing-tests module:

(Maybe rename picocli-annotation-processing-tests to picocli-tests-annotation-processing to align the name of all test modules? Also rename picocli-legacy-tests to picocli-tests-legacy.)

@remkop remkop added the theme: build An issue or change related to the build system label Jan 24, 2023
@remkop remkop mentioned this issue Jan 24, 2023
5 tasks
@remkop remkop added this to the 4.7.2 milestone Feb 5, 2023
remkop added a commit that referenced this issue Feb 5, 2023
* System.exit tests
* Environment variable tests
remkop added a commit that referenced this issue Feb 5, 2023
prevent UnsupportedOperationException in SystemLambda.withSecurityManager
@remkop remkop closed this as completed Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: build An issue or change related to the build system
Projects
None yet
Development

No branches or pull requests

1 participant