Skip to content

Commit

Permalink
fix(tests): fix flaky TestRunGameTask tests (#614)
Browse files Browse the repository at this point in the history
* chore(tests): set spf4j-slf4j-test version to 8.8.5
* chore(spellcheck): `spdx` isn't exactly a word, but `SPDX` *is* expected in the copyright header
* fix(tests): make additions to `actualHistory` all happen on the Fx application thread.

Otherwise the entries from the test thread and the application thread are subject to race conditions, making for flaky tests.
  • Loading branch information
keturn authored Nov 15, 2020
1 parent a9deb8a commit f23af05
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 7 deletions.
1 change: 1 addition & 0 deletions .idea/dictionaries/kevint.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ dependencies {
}
testImplementation "org.mockito:mockito-junit-jupiter:3.3.+"

testImplementation('org.spf4j:spf4j-slf4j-test:8.8.+') {
testImplementation('org.spf4j:spf4j-slf4j-test:8.8.5') {
because "testable logging"
}
testImplementation("org.slf4j:slf4j-api:1.7.+!!") {
Expand Down
14 changes: 8 additions & 6 deletions src/test/java/org/terasology/launcher/game/TestRunGameTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package org.terasology.launcher.game;

import com.google.common.util.concurrent.ThreadFactoryBuilder;
import javafx.application.Platform;
import javafx.concurrent.WorkerStateEvent;
import javafx.util.Pair;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -223,7 +224,8 @@ public void testSuccessEvent() throws Exception {
Happenings.TASK_COMPLETED.val()
);

final Runnable handleLineSent = () -> actualHistory.add(Happenings.PROCESS_OUTPUT_LINE.val());
final Runnable handleLineSent = () -> Platform.runLater(
() -> actualHistory.add(Happenings.PROCESS_OUTPUT_LINE.val()));

// This makes our "process," which streams out its lines and runs the callback after each.
final Process lineAtATimeProcess = new MockProcesses.OneLineAtATimeProcess(
Expand All @@ -245,11 +247,10 @@ public void testSuccessEvent() throws Exception {
// Act!
executor.submit(gameTask);

WaitForAsyncUtils.waitForFxEvents();

var actualReturnValue = gameTask.get(); // task.get blocks until it has run to completion

// Assert!
WaitForAsyncUtils.waitForFxEvents();
assertTrue(actualReturnValue);

assertIterableEquals(expectedHistory, actualHistory, renderColumns(actualHistory, expectedHistory));
Expand All @@ -271,7 +272,8 @@ public void testFastExitDoesNotResultInSuccess() {
Happenings.TASK_FAILED.val()
);

final Runnable handleLineSent = () -> actualHistory.add(Happenings.PROCESS_OUTPUT_LINE.val());
final Runnable handleLineSent = () -> Platform.runLater(
() -> actualHistory.add(Happenings.PROCESS_OUTPUT_LINE.val()));

// This makes our "process," which streams out its lines and runs the callback after each.
final Process lineAtATimeProcess = new MockProcesses.OneLineAtATimeProcess(
Expand All @@ -297,9 +299,9 @@ public void testFastExitDoesNotResultInSuccess() {
// Act!
executor.submit(gameTask);

WaitForAsyncUtils.waitForFxEvents();

var thrown = assertThrows(ExecutionException.class, gameTask::get);

WaitForAsyncUtils.waitForFxEvents();
assertThat(thrown.getCause(), instanceOf(RunGameTask.GameExitTooSoon.class));

assertIterableEquals(expectedHistory, actualHistory, renderColumns(actualHistory, expectedHistory));
Expand Down

0 comments on commit f23af05

Please sign in to comment.