Tests that don't use @RunWith(BarometerTester.class) do not always run in v0.0.7

[tdaffin]

There is a problem with the fix for the FMLSecurityException:

If you also have tests that do not use BarometerTester then they don't always run, because the last test that does run with BarometerTester causes the test process to exit.

I've pushed an example to my repo: https://github.com/jjtParadox/Barometer/compare/1.12...tdaffin:junit-bug

In the example the test com.jjtparadox.barometer.otherpackage.test.NotBarometerTest#testThatAllJUnitTestsRun does not get run unless I set systemProperty "barometer.exitWhenCompleted", false in build.gradle, but that brings back the security exception.

FYI: This is also a problem for https://github.com/CyclopsMC/IntegratedDynamics/pull/562 as IntegratedDynamics also has a bunch of jUnit only tests.

If it helps I can submit this example test as a PR.

[tdaffin]

I've just noticed that there were problems of kind before the fix for FMLSecurityException (and so they can be reproduced by setting exitWhenCompleted to false in v0.0.7). Because BarometerTester calls net.minecraft.server.MinecraftServer#initiateShutdown then the test process can exit before the non barometer tests have run.

[tdaffin]

I've been looking into this problem for a while before I posted this, so I figured I'd record my findings here:

1. There doesn't appear to be any way to cleanly stop the mincraft/forge server without causing the process to exit This in itself is a problem because then the gradle TestWorker never gets to cleanly finish it's work

2. There doesn't appear to be any way to launch the forge server without having it install FMLSecurityManager FMLSecurityManager will then not allow itself to be replaced, which the gradle TestWorker wants to do. See https://docs.gradle.org/current/userguide/java_plugin.html#sec:test_execution

3. When the gradle TestWorker tries to uninstall the FMLSecurityManager an exception is thrown that the TestWorker doesn't try to catch, so it never gets to even try to do its normal exit code, even if the sever doesn't cause the entire process to exit

To me it seems that what we really need are a couple of changes to gradle:

• Change gradle's TestWorker so that it will continue even if an exception is thrown when it tries to set the security manager to null.
• Hook into the gradle test process shutdown code so that the shutdown can be done through forge

As far as I can see though, even a fix to gradle will be a problem as ForgeGradle uses gradle 2.14 which is a really old version, so I don't know if we can even get ForgeGradle to pick up the fixes anyway.

So, I guess for now, the only to use Barometer is to make everything use @RunWith(BarometerTester)

[tdaffin]

Perhaps an easier solution would be to keep the barometer tests entirely separate from the normal gradle tests, so that we can have complete control over the test process(es)...

[tdaffin]

I'm working on a fix that runs the barometer tests in a separate process. I've made some good progress, but it isn't quite working yet: https://github.com/jjtParadox/Barometer/compare/1.12...tdaffin:junit-fix

I figured I'd log what I'd done so far to save any possible duplicated effort though

[jjtParadox]

Your findings support a lot of what I found myself when trying to fix the security exception. However, now that Barometer has a proper gradle plugin it now may be possible to have a separate test task like you mentioned.

The only thing that worries me is moving Barometer to a separate task or process might disable the nice JUnit integration with intellij, travis, etc. If you figure out your separate processes and still keep the JUnit integration, let me know!

[tdaffin]

Yep, running into problems debugging the launched process already... I'll keep chipping away at this as I'm enjoying learning some new stuff here -- gradle and especially Kotlin, but also some more of the details of how jUnit works. Could be a while before I have a PR though -- or I might just have to give up this approach entirely ;)

[jjtParadox]

If you're running into roadblocks with Kotlin, go ahead and write it in Java. I can convert it to Kotlin at a later point, as it's pretty easy to do so :smiley:

[jjtParadox]

Life got a bit busy for me so I wasn't able to work on Barometer for a while. Do you have any updates for this PR?

[tdaffin]

Not really made any worthwhile progress -- java doesn't support automatically debugging child processes and to me that just makes the approach untenable. Life also has taken some new turns for me so I've been working on other projects that are not open source. Probably not much change I'll be getting back to this anytime soon.

[jjtParadox]

No worries! I might try and use some of the stuff you posted earlier in a fix for this. I hope real life things work out for you!