-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
com.hazelcast.test.HazelcastTestSupportTest [HZG-15] #26335
Comments
Failed again on master with redhat-21: https://jenkins.hazelcast.com/job/Hazelcast-master-RedHatJDK21/18/testReport/junit/com.hazelcast.test/HazelcastTestSupportTest/ |
Failed again on master with Windows - OracleJDK21: https://jenkins.hazelcast.com/job/Hazelcast-master-Windows-OracleJDK21/4/testReport/com.hazelcast.test/HazelcastTestSupportTest/ |
Internal Jira issue: HZ-4753 |
Looks like here 2 tests are also affected due to above: |
Failed again on master with MacOS - ZuluJDK21: https://jenkins.hazelcast.com/job/Hazelcast-master-MacOS-ZuluJDK21/42/testReport/com.hazelcast.test/HazelcastTestSupportTest/ |
@burakgok could this have been introduced in https://github.com/hazelcast/hazelcast-mono/pull/1303? |
Core reflection classes were reimplemented with method handles in Java 18 (JEP 416), and consequently `HazelcastTestSupport#ENVIRONMENT` no longer works. This PR removes `HazelcastTestSupport#ENVIRONMENT` and switches to `SystemStubs#withEnvironmentVariables`, which 1. uses Instrumentation API, so not likely to be broken in a future version of Java, and 2. recovers the original environment variables after the test, but 3. doesn't support parallel test execution like `HazelcastTestSupport#ENVIRONMENT`. Fixes #26335 Why was SystemStubs chosen? 1. `HazelcastTestSupport#getEnvironmentVariables()` was an improvement over JUnit Pioneer’s implementation, and that's why it was Reflection-based. However, the security measures against reflective hacks are getting hardened in each Java version, as they try to make people use Instrumentation API instead. 2. `Mockito#mockStatic` offers thread-local auto-closing mocks of static utilities, but it doesn’t support mocking `System#getEnv`. 3. PowerMock supports it, but we don’t have it as a dependency. Also, it imposes its own JUnit runner. There is way to delegate the actual test execution to a third party runner, but it may create problems with our complex parameterized runners. 4. `SystemStubs#withEnvironmentVariables` uses Instrumentation API, and we already have a dependency to SystemStubs. **Note:** Both `Mockito#mockStatic` and `SystemStubs#withEnvironmentVariables` uses ByteBuddy under the hood. The reason why Mockito doesn't support mocking `System` utilities unlike SystemStubs is that Mockito tries to offer a consistent API for static mocks and replacing the original `System` class with an instrumented one in a thread-local classloader seems impossible because of the involvement of `System` in classloading. **Note:** This PR had a Reflection-based solution initially, which can be seen from the following: https://github.com/hazelcast/hazelcast-mono/blob/3f009858/hazelcast/hazelcast/src/test/java/com/hazelcast/test/ReflectionUtils.java [JEP 416]: https://openjdk.org/jeps/416 GitOrigin-RevId: fe2dc8682097fc8132bbeca37ab4e9e9ccfef5de
master (commit 4e2c5389247b64991932023ffaa91e6ec0f5bf24)
Failed on MacOS - ZuluJDK21: https://jenkins.hazelcast.com/job/Hazelcast-master-MacOS-ZuluJDK21/41/testReport/junit/com.hazelcast.test/HazelcastTestSupportTest/
Stacktrace:
Standard output:
Standard output can be found here - https://s3.console.aws.amazon.com/s3/buckets/j-artifacts/Hazelcast-master-MacOS-ZuluJDK21/41/
The text was updated successfully, but these errors were encountered: