[tester test] Avoid use of setSecurityManager()

[kriegfrj]

The current tester tests uses a custom security manager to intercept calls to System.exit() so that they can verify correct behaviour.

In Java 17, setSecurityManager() is deprecated. We're getting warnings like this:

WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by aQute.tester.junit.platform.test.GogoShellTests (file:/home/runner/work/bnd/bnd/biz.aQute.tester.test/bin_test/)
WARNING: Please consider reporting this to the maintainers of aQute.tester.junit.platform.test.GogoShellTests
WARNING: System::setSecurityManager will be removed in a future release

We will need to look at alternatives at some point.

One possibility I have considered is using https://github.com/webcompere/system-stubs, which has a catchSystemExit() implementation that uses mockito-inline under-the-hood.

[bjhargrave]

@pkriens has stated that there is a way to invoke the Bnd launcher that cause it to not call System.exit but instead just return. Perhaps Peter can shed some light on this here.

[kriegfrj]

@pkriens has stated that there is a way to invoke the Bnd launcher that cause it to not call System.exit but instead just return. Perhaps Peter can shed some light on this here.

That might help, but if System.exit() behaviour is still supported behaviour then it is nice to test that properly too (eg, the correct number of failed tests is returned), which the current tests do.

[kriegfrj]

One possibility I have considered is using https://github.com/webcompere/system-stubs, which has a catchSystemExit() implementation that uses mockito-inline under-the-hood.

I misunderstood System Stubs - catchSystemExit() still uses a security manager.

The only reliable way to intercept calls without a security manager would be using instrumentation/code weaving.

[pkriens]

Look at the launcher to not exit:

 public static void main(String[] args) {
    try {
        int exitcode = 0;
        try {

            exitcode = Launcher.run(args);
        } catch (FrameworkRestart r) {
            throw r;
        } catch (Throwable t) {
            exitcode = 127;
            // Last resort ... errors should be handled lower
            t.printStackTrace(System.err);
        }

        // We exit, even if there are non-daemon threads active
        // though we've reported those
        if (exitcode != LauncherConstants.RETURN_INSTEAD_OF_EXIT)
            System.exit(exitcode);
    } finally {
        System.out.println("gone");
    }
}

[pkriens]

@kriegfrj ok if I fix this by not calling the exit() method?

[kriegfrj]

Yes, I think that is ok. Though I can't remember if the test code uses the launcher - I think it does (via launchpad).

Usually, I do not like tests that force the code-under-test to take a different code path when being tested vs production. It is a recipe for code that passes tests only to fail in production. Which is why I went to lengths originally to find a way to run the code without modification. But until there is an alternative in the VM to prevent exiting (without a deprecation warning) I do not think that we have an alternative other than to alter the code path to prevent exiting.

[HannesWell]

The JEP 411 mentions that there are new API to be developed as a replacement for intercepting System::exit, yet non has arrived yet: https://bugs.openjdk.org/browse/JDK-8199704

Since version 5, Mockito can also mock static methods out out the box. However something like the following code does not work.

@Test
void testSystemExit() throws Exception {
    try (MockedStatic<System> systemMock = Mockito.mockStatic(System.class);) {
        systemMock.when(() -> System.exit(0)).thenReturn(null);
        System.exit(1);
    }
    System.out.println("Still running");
}

It fails with an exception, 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, which is not surprising since its doc states

Note: We recommend against mocking static methods of classes in the standard library orclasses used by
custom class loaders used to execute the block with the mocked class. A mockmaker might forbid mocking 
static methods of know classes that are known to cause problems.Also, if a static method is a JVM-intrinsic, 
it cannot typically be mocked even if not explicitly forbidden. 

and mocking the System class seems to be one of those cases.

So I think you either have to try the suggested system-stubs library or introduce an indirection that allows to prevent calling System.exit() during tests. For example there could be a static method exitJVM() that calls System.exit(). There could either be a flag that prevents the System.exit() and can be turned on during tests or that method could be statically mocked with mockito to do nothing.