<@U7BB83NPR> <@U4Z1U6LJK> Thanks guys :slightly_sm...

# tornadofx

[edvin]

@Carlton Whitehead @kastork Thanks guys 🙂 I'm sick, so just checking in. Should I merge the PR now or wait?

[Carlton Whitehead]

I recommend we wait to merge until we can get the tests to pass reliably in a range of environments. If it is merged now, the tests will run but likely will be flaky

[Carlton Whitehead]

I put the PR up hoping people can check it in some different environments.

[edvin]

OK, I can test on Windows/my environment, hopefully tomorrow.

[kastork]

I'll try to get to it a little later today.

[kastork]

mvn test on macos cmd line https://gist.github.com/kastork/624030a1c864ce8fff34184ddadd1620

[kastork]

When running via right click on the test folder in InteliJ, I see some gui action (but not as much as I'm used to).

[kastork]

Then ChildInterceptorTest fails https://gist.github.com/kastork/704f08156bea3488d7c21b13b0ad0068

[Carlton Whitehead]

Thanks @kastork . That is the same as I'm seeing locally on my Linux laptop. The good news is the tests are executing now where they weren't at all before, and only one is flaky. I'll take a closer look at that flaky one this weekend

[edvin]

Thanks for taking the lead on this guys. Looking at ChildInterceptorTest now.

[edvin]

The assert parameters are given in the wrong order also.

[edvin]

For example assertEquals(FX.childInterceptors.size, 2) should be assertEquals(2, FX.childInterceptors.size) - expected first, then actual

[edvin]

It looks like app.init is run three times in the environment where it's failing. Every time you instantiate an App subclass in the same JVM and you have interceptors configured via the service loader, this would happen.

[edvin]

Let me come up with a quick fix to check for this.

[edvin]

I have to pick up my kid from a birthday party, then I'll commit it. Pretty sure it will take care of the issue.

[edvin]

@kastork Can you try to change addInterceptor to this:

[edvin]

fun addChildInterceptor(interceptor: ChildInterceptor) {
if (childInterceptors.none { it::class == interceptor::class })
childInterceptors.add(interceptor)
}

[edvin]

And rerun the test on your mac?

[kastork]

sure

[kastork]

Tests run: 331, Failures: 0, Errors: 0, Skipped: 0

[kastork]

Saw some GUI activity, but again, I seem to remember there being a lot more than what I'm seeing now.

[kastork]

Also, there's no test failure but I see a stack trace

Feb 02, 2018 10:33:31 AM tornadofx.CssSelectionBlock <init>
WARNING: CSS rule caused an error
kotlin.TypeCastException: null cannot be cast to non-null type javafx.scene.paint.Color

[edvin]

@kastork That warning is supposed to be there, it's actually part of a test to check for failing css

[edvin]

@kastork The if will not be needed, the correct way to solve this is to remove the call to importChildInterceptors() from App.init() and put it in the FX.Companion init instead. I'm committing this fix now.

[edvin]

Committed 🙂

[edvin]

The "flakyness" is probably a result of tests executing in different order, but tests across different test classes shouldn't rely on order. In fact, it revealed a flaw in the framework, so that worked out great 🙂

[kastork]

Right.... if things are flaky because of execution order, then something is amiss.