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

com.hazelcast.test.HazelcastTestSupportTest [HZG-15] #26335

Closed
sumnerib opened this issue May 16, 2024 · 6 comments
Closed

com.hazelcast.test.HazelcastTestSupportTest [HZG-15] #26335

sumnerib opened this issue May 16, 2024 · 6 comments
Assignees
Labels
Milestone

Comments

@sumnerib
Copy link
Contributor

master (commit 4e2c5389247b64991932023ffaa91e6ec0f5bf24)

Failed on MacOS - ZuluJDK21: https://jenkins.hazelcast.com/job/Hazelcast-master-MacOS-ZuluJDK21/41/testReport/junit/com.hazelcast.test/HazelcastTestSupportTest/

Stacktrace:
java.lang.InternalError: java.lang.IllegalAccessException: static final field has no write access: com.hazelcast.test.HazelcastTestSupportTest$TestClass.publicStaticFinalField/java.lang.Object/putStatic, from class java.lang.Object (module java.base)
	at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:167)
	at java.base/jdk.internal.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:145)
	at java.base/java.lang.reflect.Field.acquireOverrideFieldAccessor(Field.java:1200)
	at java.base/java.lang.reflect.Field.getOverrideFieldAccessor(Field.java:1169)
	at java.base/java.lang.reflect.Field.set(Field.java:836)
	at com.hazelcast.test.HazelcastTestSupport.setFieldValue(HazelcastTestSupport.java:1516)
	at com.hazelcast.test.HazelcastTestSupport.setFieldValue(HazelcastTestSupport.java:1496)
	at com.hazelcast.test.HazelcastTestSupportTest.test_setPublicStaticFinalField(HazelcastTestSupportTest.java:34)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at com.hazelcast.test.FailOnTimeoutStatement$CallableStatement.call(FailOnTimeoutStatement.java:115)
	at com.hazelcast.test.FailOnTimeoutStatement$CallableStatement.call(FailOnTimeoutStatement.java:107)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.IllegalAccessException: static final field has no write access: com.hazelcast.test.HazelcastTestSupportTest$TestClass.publicStaticFinalField/java.lang.Object/putStatic, from class java.lang.Object (module java.base)
	at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:894)
	at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectField(MethodHandles.java:3597)
	at java.base/java.lang.invoke.MethodHandles$Lookup.unreflectSetter(MethodHandles.java:3588)
	at java.base/java.lang.invoke.MethodHandleImpl$1.unreflectField(MethodHandleImpl.java:1632)
	at java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newFieldAccessor(MethodHandleAccessorFactory.java:145)
	... 12 more

Standard output:
Finished Running Test: test_setPrivateStaticField in 0.001 seconds.
Started Running Test: test_setPublicStaticFinalField
02:30:30,218  INFO |test_setPublicStaticFinalField| - [HazelcastTestSupport] main - Shutting down node factory as @After action
BuildInfo right after test_setPublicStaticFinalField(com.hazelcast.test.HazelcastTestSupportTest): BuildInfo{version='5.5.0-SNAPSHOT', build='20240516', buildNumber=20240516, revision=, enterprise=false, serializationVersion=1}
Hiccups measured while running test 'test_setPublicStaticFinalField(com.hazelcast.test.HazelcastTestSupportTest):'
02:30:30, accumulated pauses: 88 ms, max pause: 69 ms, pauses over 1000 ms: 0


No metrics recorded during the test

Standard output can be found here - https://s3.console.aws.amazon.com/s3/buckets/j-artifacts/Hazelcast-master-MacOS-ZuluJDK21/41/

@sumnerib sumnerib added Type: Test-Failure Source: Internal PR or issue was opened by an employee Team: DevInfra labels May 16, 2024
@sumnerib sumnerib added this to the 5.5.0 milestone May 16, 2024
@sumnerib
Copy link
Contributor Author

@Patras3
Copy link
Contributor

Patras3 commented May 21, 2024

@github-actions github-actions bot changed the title com.hazelcast.test.HazelcastTestSupportTest com.hazelcast.test.HazelcastTestSupportTest [HZ-4753] May 21, 2024
Copy link
Contributor

Internal Jira issue: HZ-4753

@Patras3
Copy link
Contributor

Patras3 commented May 21, 2024

Looks like here 2 tests are also affected due to above:
Failed again on master with Windows - OracleJDK21: https://jenkins.hazelcast.com/job/Hazelcast-master-Windows-OracleJDK21/4/testReport/com.hazelcast.internal.util.phonehome/PhoneHomeIntegrationTest/

@Patras3
Copy link
Contributor

Patras3 commented May 21, 2024

@JackPGreen
Copy link
Contributor

JackPGreen commented May 21, 2024

@burakgok could this have been introduced in https://github.com/hazelcast/hazelcast-mono/pull/1303?

@burakgok burakgok self-assigned this May 21, 2024
@burakgok burakgok changed the title com.hazelcast.test.HazelcastTestSupportTest [HZ-4753] com.hazelcast.test.HazelcastTestSupportTest [HZG-15] May 22, 2024
devOpsHazelcast pushed a commit that referenced this issue May 23, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants