Spring Use SpringProperties in native image detection

While writing tests for @EnabledOnNative and @DisabledOnNative in Spring Native (https://github.com/spring-projects-experimental/spring-native/pull/1460), I encountered it is very difficult to mock the native detection result - NativeDetector.inNativeImage().

This is because it performs a system property check at the class initialization and keeps the result in a static final variable.

In this PR, I changed the detection logic to lookup SpringProperties, similar to AotModeDetector does in Spring Native.

This way, tests that need to modify the native-detection-result can interact with SpringProperties to dynamically change the value.

Comment From: sbrannen

Hi @ttddyy,

Thanks for the proposal.

Since the system property in question is not one owned by the Spring Framework, we do not feel comfortable looking up its value via SpringProperties.

Instead, we suggest you find an alternative approach to testing this.

For example, the following test class uses a custom ClassLoader that allows you to set the org.graalvm.nativeimage.imagecode before the NativeDetector class is initialized.

import java.lang.reflect.Method;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.junit.jupiter.api.Test;
import org.springframework.util.ReflectionUtils;

import static org.assertj.core.api.Assertions.assertThat;

/**
 * Tests for {@link NativeDetector}.
 *
 * @author Sam Brannen
 * @author Tadaya Tsuyukubo
 */
class NativeDetectorTests {

    private static final String CLASS_NAME = "org.springframework.core.NativeDetector";

    @Test
    void inNativeImage() throws Exception {
        assertThat(NativeDetector.inNativeImage()).isFalse();
        try {
            System.setProperty("org.graalvm.nativeimage.imagecode", "test");
            Class<?> clazz = new TestClassLoader(getClass().getClassLoader()).loadClass(CLASS_NAME, false);
            Method method = clazz.getMethod("inNativeImage");
            Object result = ReflectionUtils.invokeMethod(method, null);
            assertThat(result).asInstanceOf(InstanceOfAssertFactories.BOOLEAN).isTrue();
        }
        finally {
            System.clearProperty("org.graalvm.nativeimage.imagecode");
        }
        assertThat(NativeDetector.inNativeImage()).isFalse();
    }


    private static class TestClassLoader extends OverridingClassLoader {

        TestClassLoader(ClassLoader parent) {
            super(parent);
        }

        @Override
        protected boolean isEligibleForOverriding(String className) {
            return CLASS_NAME.equals(className);
        }

    }

}

Hopefully you can use a similar approach in your tests for https://github.com/spring-projects-experimental/spring-native/pull/1460.

Please let us know if that works for you.

Comment From: ttddyy

@sbrannen Thanks for the suggestion. Using a throwaway classloader is such a neat technique. It works for testing NativeDetector.inNativeImage() method itself.

However, in my case, the test does not directly call the NativeDetector.inNativeImage(). The test target logic(junit extension classes launched programmatically) calls NativeDetector.inNativeImage(). So, I cannot directly interact with the throwaway classloader to mock the response.

Comment From: sbrannen

Hi @ttddyy,

That example was intended to be a proof of concept.

Here's a fully working test class for your DisabledOnNativeCondition.

import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Constructor;
import java.util.Optional;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ConditionEvaluationResult;
import org.junit.jupiter.api.extension.ExecutionCondition;
import org.junit.jupiter.api.extension.ExtensionContext;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.InstanceOfAssertFactories.STRING;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

/**
 * Unit tests for {@link DisabledOnNativeCondition}.
 *
 * @author Sam Brannen
 */
class DisabledOnNativeConditionTests {

    private static final String IMAGECODE_PROPERTY_NAME = "org.graalvm.nativeimage.imagecode";
    private static final String NATIVE_DETECTOR_CLASS_NAME = "org.springframework.core.NativeDetector";
    private static final String DISABLED_ON_NATIVE_CONDITION_CLASS_NAME = "org.springframework.core.DisabledOnNativeCondition";


    @BeforeEach
    void preconditions() {
        assertThat(NativeDetector.inNativeImage()).isFalse();
    }

    @AfterEach
    void postconditions() {
        System.clearProperty(IMAGECODE_PROPERTY_NAME);
        assertThat(NativeDetector.inNativeImage()).isFalse();
    }

    @Test
    void enabledOnNativeImageWhenImageCodePropertyIsNotSet() throws Exception {
        System.clearProperty(IMAGECODE_PROPERTY_NAME);

        ConditionEvaluationResult result = getConditionEvaluationResult();
        assertThat(result).isNotNull();
        assertThat(result.isDisabled()).isFalse();
        assertThat(result.getReason()).get().asInstanceOf(STRING).matches("^.+ is enabled on non native image$");
    }

    @Test
    void disabledOnNativeImageWhenImageCodePropertyIsSet() throws Exception {
        System.setProperty(IMAGECODE_PROPERTY_NAME, "test");

        ConditionEvaluationResult result = getConditionEvaluationResult();
        assertThat(result).isNotNull();
        assertThat(result.isDisabled()).isTrue();
        assertThat(result.getReason()).get().asInstanceOf(STRING).matches("^.+ is disabled on native image$");
    }

    private ConditionEvaluationResult getConditionEvaluationResult() throws Exception {
        TestClassLoader testClassLoader = new TestClassLoader(getClass().getClassLoader());
        Class<?> disbledOnNativeConditionClass = testClassLoader.loadClass(DISABLED_ON_NATIVE_CONDITION_CLASS_NAME, false);
        Constructor<?> constructor = disbledOnNativeConditionClass.getDeclaredConstructor();
        constructor.setAccessible(true);
        ExecutionCondition condition = (ExecutionCondition) constructor.newInstance();

        ExtensionContext context = mock(ExtensionContext.class);
        when(context.getElement()).thenReturn(Optional.of(mock(AnnotatedElement.class)));

        return condition.evaluateExecutionCondition(context);
    }


    private static class TestClassLoader extends OverridingClassLoader {

        TestClassLoader(ClassLoader parent) {
            super(parent);
        }

        @Override
        protected boolean isEligibleForOverriding(String className) {
            return NATIVE_DETECTOR_CLASS_NAME.equals(className) ||
                    DISABLED_ON_NATIVE_CONDITION_CLASS_NAME.equals(className);
        }

    }

}

Feel free to use that (or something based on that) in your PR for Spring Native.

Comment From: ttddyy

@sbrannen thanks for your time putting on this.

Initially, I was thinking to test the @EnabledOnNative annotation itself by annotating classes and methods and launching them programmatically. So, it was more like an integration test rather than testing the condition implementation. But since it is a unit test, I believe testing the condition should be sufficient for the native check annotations. I'll apply the provided solution.

Comment From: sbrannen

Hi @ttddyy,

I'm glad that works for you.

However, after rethinking it, I think the approach I proposed is unnecessarily complex for the task at hand. A unit test certainly suffices, and we can achieve that without jumping through hoops using a custom ClassLoader and reflection.

I'm not sure why I didn't immediately think of this, but... my usual approach for testing something that depends on something else that you cannot readily mock is to introduce a package-private (or protected) method which interacts with that "unmockable" thing (such as the OS environment or a static utility like NativeDetector). Then you can mock the package-private method (or rather override it).

For example, that's how I test the @EnabledIfEnvironmentVariable support in JUnit Jupiter.

https://github.com/junit-team/junit5/blob/89f6bbce2041ef29580df59ee86a9471a7698f3b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/condition/EnabledIfEnvironmentVariableCondition.java#L63-L72

https://github.com/junit-team/junit5/blob/89f6bbce2041ef29580df59ee86a9471a7698f3b/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/condition/EnabledIfEnvironmentVariableConditionTests.java#L34-L43

In your case, I would introduce the following method in your conditions and override it in your tests.

    /**
     * Determine if we are running in a native image.
     *
     * <p>The default implementation delegates to
     * {@link NativeDetector#inNativeImage()}. Can be overridden in a subclass for
     * testing purposes.
     */
    boolean inNativeImage() {
        return NativeDetector.inNativeImage();
    }

Comment From: ttddyy

@sbrannen Yes, I agree with the suggestion. I also take that approach for my usual tests - slightly modify the main code for tests. Thanks for the advice. I have updated my code on the native side accordingly.