system lambda jdk 16 compatibility issue

[SylvainJuge]

We use system-lambda for testing custom environment variables.
There is a known compatibility issue with JDK16 that makes some of our tests to fail.

There is no reliable work-around for now, the only option would be to modify our test implementation (and possibly use another way to mock dependencies on System class, at least to set environment variables.

Known affected test(s)

• co.elastic.apm.agent.impl.payload.ContainerInfoTest.testKubernetesDownwardApi

How to reproduce:

• simply run tests with JDK 16

Work-around (not usable outside of testing)

• add --add-opens=java.base/java.util=ALL-UNNAMED into JVM arguments

Stack trace:

java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.util.Map java.util.Collections$UnmodifiableMap.m accessible: module java.base does not "opens java.util" to unnamed module @66133adc

	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:177)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:171)
	at com.github.stefanbirkner.systemlambda.SystemLambda$WithEnvironmentVariables.getFieldValue(SystemLambda.java:1275)
	at com.github.stefanbirkner.systemlambda.SystemLambda$WithEnvironmentVariables.getEditableMapOfVariables(SystemLambda.java:1235)
	at com.github.stefanbirkner.systemlambda.SystemLambda$WithEnvironmentVariables.restoreOriginalVariables(SystemLambda.java:1213)
	at com.github.stefanbirkner.systemlambda.SystemLambda$WithEnvironmentVariables.execute(SystemLambda.java:1128)
	at co.elastic.apm.agent.impl.payload.ContainerInfoTest.testKubernetesDownwardApi(ContainerInfoTest.java:137)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:567)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at .....

[felixbarny]

We could use the static mocking feature of Mockito 3.4.0+ to work around that: https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#48

This requires the mockito-inline dependency.

Example:

 try (MockedStatic mocked = mockStatic(System.class)) {
    Map<String, String> mockedEnv = new HashMap(System.getenv());
    mockedEnv.put("foo", "bar");
    mocked.when(System::getenv).thenReturn(mockedEnv);
    assertEquals("bar", System.getenv().get("foo"));
 }

[eyalkoren]

org.mockito.exceptions.base.MockitoException: It is not possible to mock static methods of java.lang.System to avoid interfering with class loading what leads to infinite loops

[felixbarny]

Hmm.. Then two more ideas.

1. Try to convince the Mockito folks to make the java.lang.System exception more fine grained. For example to only allow mocking getenv or to only disallow the problematic methods.
2. Just write a custom instrumentation that only applies to tests that can manipulate the return value of System::getenv. ContainerInfoTest would then extend from AbstractInstrumentationTest.

[SylvainJuge]

Alternatively, we could also create a very small abstraction over the calls to System class when retrieving environment variables.
When testing, we can use an alternate implementation that allows setting arbitrary values.

[felixbarny]

Yes, that's probably the simplest and cleanest approach 🙂

[eyalkoren]

I actually took my chances with the instrumentation approach, mainly because it is much more fun, but also I think it is actually cleaner than changing production code for testing purposes.
I hope it doesn't cause a mess 🤞

[eyalkoren]

Closed by #2195