-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Game Runner improvements (take 2) #597
Conversation
This is a slf4j provider made to write test assertions against. But there can be only one slf4j provider on your classpath at the same time, otherwise who knows which will be used. the remal.component-metadata plugin identifies a lot of these sorts of conflicts so they don't happen silently. but that doesn't have spf4j-slf4j-test in its list of things it knows about, so we also need to specify that. once you've identified conflicts, you have to resolve them, which takes ten lines for some reason, so I put it in a function. debugging wasn't working very well on scripts included with `apply from`, and that's how I ended up writing a gradle plugin.
with slight refactor to avoid mocking static method on Thread.
…ibility to satisfy both app and test code.
The more I look at it the more it circles back to the current design.
like `hasItems` but it checks from one collection you pass to it, not each one of a variable number of arguments.
so other tests can use jupiter Extension
We use TestFX so the JavaFX Task has an Application Thread to run in.
Replace the GameStarter Interface with Callable<Process>.
…if it exits with error
We don't want thread cancellation to terminate the process.
…tStream. 💭 "it'll be easy," they said.
…(or at least try to).
we don't use the annotation parts of spf4j-slf4j-test enough to keep Vintage around
This does is a spec change, but the best way to avoid this "test failure won't show us TRACE output unless we use the @CollectLogs decorator" is to not rely on TRACE output. Especially since we want game output more visible anyway, for troubleshooting on launch failure.
sort of. this thing.
copied from upstream
without this, "quit launcher after start" could be frustrating.
otherwise the logger shows `null`
This pull request introduces 1 alert when merging d6108dd into 5a20d1a - view on LGTM.com new alerts:
|
There seem to be at least a couple tests with intermittent failures, including
These are failing in a very similar way, so there's likely a common cause either in the implementation or the way we're doing the tests. That An intermittent failure suggests a race condition. I thought I built those test cases to be free of race conditions, but apparently not! Also worth noting on the testFastExit failure is that the assertion it's failing on, comparing their event logs, is after it passed the assertion that says the testSuccessEvent is different in that there isn't an exception thrown. We should add the renderer that prints the two lists in columns to that one. It says it didn't see TASK_VALUE_SET when it expected it, but I wonder if it's just ended up switching its entries at spots 2 and 3. Unconfirmed SpeculationI have a hunch: the events we record from property listeners or javafx event handlers are written from a different thread than the lineSent Not sure yet how we would handle that. We want to somehow synchronize things so that the next line isn't recorded until after any potential event handlers responding to the previous line have finished? The fastExit test isn't exactly the same in that way, but still could be a race between |
replaced by #600 (moved PR branch to org repo, but a PR can't change its source branch.) |
#571 was reverted due to failing tests in CI. Let's fix that here!
(see #571 for overall branch feature description)
Outstanding:
Optional: