Skip to content
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.

Sign up for GitHub

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

Refactor Continuous Testing JSON/RPC #43119

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Refactor Continuous Testing JSON/RPC #43119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestClassResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,39 @@
import java.util.ArrayList;
import java.util.List;

public class TestClassResult implements Comparable<TestClassResult> {
import io.quarkus.dev.testing.results.TestClassResultInterface;
import io.quarkus.dev.testing.results.TestResultInterface;

public class TestClassResult implements TestClassResultInterface {
final String className;
final List<TestResult> passing;
final List<TestResult> failing;
final List<TestResult> skipped;
final long latestRunId;
final long time;

public TestClassResult(String className, List<TestResult> passing, List<TestResult> failing, List<TestResult> skipped,
public TestClassResult(
String className,
List<TestResult> passing,
List<TestResult> failing,
List<TestResult> skipped,
long time) {
this.className = className;
this.passing = passing;
this.failing = failing;
this.skipped = skipped;
this.time = time;
long runId = 0;
for (TestResult i : passing) {
runId = Math.max(i.runId, runId);
for (TestResultInterface i : passing) {
runId = Math.max(i.getRunId(), runId);
}
for (TestResult i : failing) {
runId = Math.max(i.runId, runId);
for (TestResultInterface i : failing) {
runId = Math.max(i.getRunId(), runId);
}
latestRunId = runId;
}

@Override
public String getClassName() {
return className;
}
Expand All @@ -53,15 +61,12 @@ public long getTime() {
}

@Override
public int compareTo(TestClassResult o) {
return className.compareTo(o.className);
}

public List<TestResult> getResults() {
List<TestResult> ret = new ArrayList<>();
ret.addAll(passing);
ret.addAll(failing);
ret.addAll(skipped);
return ret;
}

}
27 changes: 26 additions & 1 deletion core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import org.junit.platform.engine.TestExecutionResult;
import org.junit.platform.engine.UniqueId;

public class TestResult {
import io.quarkus.dev.testing.results.TestResultInterface;

public class TestResult implements TestResultInterface {

final String displayName;
final String testClass;
Expand Down Expand Up @@ -49,18 +51,22 @@ public TestExecutionResult getTestExecutionResult() {
return testExecutionResult;
}

@Override
public List<String> getLogOutput() {
return logOutput;
}

@Override
public String getDisplayName() {
return displayName;
}

@Override
public String getTestClass() {
return testClass;
}

@Override
public List<String> getTags() {
return tags;
}
Expand All @@ -69,23 +75,42 @@ public UniqueId getUniqueId() {
return uniqueId;
}

@Override
public boolean isTest() {
return test;
}

@Override
public String getId() {
return uniqueId.toString();
}

@Override
public long getRunId() {
return runId;
}

@Override
public long getTime() {
return time;
}

@Override
public List<Throwable> getProblems() {
return problems;
}

@Override
public boolean isReportable() {
return reportable;
}

@Override
public State getState() {
return switch (testExecutionResult.getStatus()) {
case FAILED -> State.FAILED;
case ABORTED -> State.SKIPPED;
case SUCCESSFUL -> State.PASSED;
};
}
}
25 changes: 20 additions & 5 deletions core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestRunResults.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import java.util.Map;

import io.quarkus.deployment.dev.ClassScanResult;
import io.quarkus.dev.testing.results.TestResultInterface;
import io.quarkus.dev.testing.results.TestRunResultsInterface;

public class TestRunResults {
public class TestRunResults implements TestRunResultsInterface {

/**
* The run id
Expand All @@ -28,7 +30,7 @@ public class TestRunResults {
private final long started;
private final long completed;

private final Map<String, TestClassResult> results;
private final Map<String, ? extends TestClassResult> results;
private final Map<String, TestClassResult> currentFailing = new HashMap<>();
private final Map<String, TestClassResult> historicFailing = new HashMap<>();
private final Map<String, TestClassResult> currentPassing = new HashMap<>();
Expand Down Expand Up @@ -58,9 +60,9 @@ public TestRunResults(long id, ClassScanResult trigger, boolean full, long start
long currentFailedCount = 0;
long currentSkippedCount = 0;
for (Map.Entry<String, TestClassResult> i : results.entrySet()) {
passedCount += i.getValue().getPassing().stream().filter(TestResult::isTest).count();
failedCount += i.getValue().getFailing().stream().filter(TestResult::isTest).count();
skippedCount += i.getValue().getSkipped().stream().filter(TestResult::isTest).count();
passedCount += i.getValue().getPassing().stream().filter(TestResultInterface::isTest).count();
failedCount += i.getValue().getFailing().stream().filter(TestResultInterface::isTest).count();
skippedCount += i.getValue().getSkipped().stream().filter(TestResultInterface::isTest).count();
currentPassedCount += i.getValue().getPassing().stream().filter(s -> s.isTest() && s.getRunId() == id).count();
currentFailedCount += i.getValue().getFailing().stream().filter(s -> s.isTest() && s.getRunId() == id).count();
currentSkippedCount += i.getValue().getSkipped().stream().filter(s -> s.isTest() && s.getRunId() == id).count();
Expand Down Expand Up @@ -98,6 +100,7 @@ public TestRunResults(long id, ClassScanResult trigger, boolean full, long start
this.currentSkippedCount = currentSkippedCount;
}

@Override
public long getId() {
return id;
}
Expand All @@ -110,6 +113,7 @@ public boolean isFull() {
return full;
}

@Override
public Map<String, TestClassResult> getResults() {
return Collections.unmodifiableMap(results);
}
Expand All @@ -130,14 +134,17 @@ public Map<String, TestClassResult> getHistoricPassing() {
return historicPassing;
}

@Override
public long getStartedTime() {
return started;
}

@Override
public long getCompletedTime() {
return completed;
}

@Override
public long getTotalTime() {
return completed - started;
}
Expand All @@ -154,34 +161,42 @@ public List<TestClassResult> getSkipped() {
return skipped;
}

@Override
public long getPassedCount() {
return passedCount;
}

@Override
public long getFailedCount() {
return failedCount;
}

@Override
public long getSkippedCount() {
return skippedCount;
}

@Override
public long getCurrentPassedCount() {
return currentPassedCount;
}

@Override
public long getCurrentFailedCount() {
return currentFailedCount;
}

@Override
public long getCurrentSkippedCount() {
return currentSkippedCount;
}

@Override
public long getTotalCount() {
return getPassedCount() + getFailedCount() + getSkippedCount();
}

@Override
public long getCurrentTotalCount() {
return getCurrentPassedCount() + getCurrentFailedCount() + getCurrentSkippedCount();
}
Expand Down
16 changes: 16 additions & 0 deletions core/devmode-spi/src/main/java/io/quarkus/dev/testing/results/TestClassResultInterface.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package io.quarkus.dev.testing.results;

import java.util.List;

public interface TestClassResultInterface extends Comparable<TestClassResultInterface> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify the purpose of all these new interfaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test results are stored into objects of type TestRunResults, TestClassResult and TestResult. They are returned to the ContinuousTestingJsonRPCService as a generic java.lang.Object and there transformed to JSON - then sent to the browser for UI rendering. Because my intention was to avoid passing the objects through the RPC service without any API awareness, validation..., the RPC service needs the objects typified.
Unfortunately, the module, where the JSON RPC service is placed, does not have a compile dependency to the module where the Test result types are declared. (And I did not want to change this, because this might lead to a cyclic dependency.) So I declared the interfaces (whose names might sound uncreative, because I did not want to change the implementation classes' names to avoid too much diffs) in one module where both have access to. (I'm not sure if this is the correct module, but I found the ContinuousTestingSharedStateManager there, so I think it is intended so)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Furthermore, the interfaces have the advantage that we can be aware of which informationen in required for the Continuous Testing UI, and which information is not. (that is not part of the interface) This makes further refactoring easier, because we now only have compile-time dependencies. (The test results are also used for printing out in the console for example.)

Copy link
Contributor Author

@ueberfuhr ueberfuhr Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is unclear for me: Why are the classes TestRunResults, TestClassResult and TestResultplaced into a deployment module (core-deployment)? They could also be placed into development-mode-spi, where the interfaces are now? This might avoid the need of the interfaces. (see the class diagram)


String getClassName();

List<? extends TestResultInterface> getResults();

@Override
default int compareTo(TestClassResultInterface o) {
return getClassName().compareTo(o.getClassName());
}

}
34 changes: 34 additions & 0 deletions core/devmode-spi/src/main/java/io/quarkus/dev/testing/results/TestResultInterface.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.quarkus.dev.testing.results;

import java.util.List;

public interface TestResultInterface {
List<String> getLogOutput();

String getDisplayName();

String getTestClass();

List<String> getTags();

boolean isTest();

String getId();

long getRunId();

long getTime();

List<Throwable> getProblems();

boolean isReportable();

State getState();

enum State {
PASSED,
FAILED,
SKIPPED
}

}
31 changes: 31 additions & 0 deletions core/devmode-spi/src/main/java/io/quarkus/dev/testing/results/TestRunResultsInterface.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package io.quarkus.dev.testing.results;

import java.util.Map;

public interface TestRunResultsInterface {
long getId();

Map<String, ? extends TestClassResultInterface> getResults();

long getStartedTime();

long getCompletedTime();

long getTotalTime();

long getPassedCount();

long getFailedCount();

long getSkippedCount();

long getCurrentPassedCount();

long getCurrentFailedCount();

long getCurrentSkippedCount();

long getTotalCount();

long getCurrentTotalCount();
}
3 changes: 2 additions & 1 deletion ...deployment/src/main/java/io/quarkus/devui/deployment/menu/ContinuousTestingProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.quarkus.deployment.dev.testing.TestRunResults;
import io.quarkus.deployment.dev.testing.TestSupport;
import io.quarkus.dev.spi.DevModeType;
import io.quarkus.dev.testing.results.TestRunResultsInterface;
import io.quarkus.devui.deployment.InternalPageBuildItem;
import io.quarkus.devui.runtime.continuoustesting.ContinuousTestingJsonRPCService;
import io.quarkus.devui.runtime.continuoustesting.ContinuousTestingRecorder;
Expand Down Expand Up @@ -237,7 +238,7 @@ private void registerGetStatusMethod(LaunchModeBuildItem launchModeBuildItem, Bu
}

private void registerGetResultsMethod(LaunchModeBuildItem launchModeBuildItem, BuildTimeActionBuildItem actions) {
actions.addAction("getResults", ignored -> {
actions.<TestRunResultsInterface> addAction("getResults", ignored -> {
try {
Optional<TestSupport> ts = TestSupport.instance();
if (testsDisabled(launchModeBuildItem, ts)) {
Expand Down
Loading
Loading