Skip to content

Report problematic selector resolution results as discovery issues #4389

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

Merged
merged 1 commit into from
Mar 17, 2025
Merged

Report problematic selector resolution results as discovery issues #4389

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
28 changes: 22 additions & 6 deletions ...stractLauncherDiscoveryListenerTests.java → ...nit/platform/fakes/FaultyTestEngines.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,20 @@
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.platform.launcher.listeners.discovery;
package org.junit.platform.fakes;

import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.EngineDiscoveryRequest;
import org.junit.platform.engine.ExecutionRequest;
import org.junit.platform.engine.SelectorResolutionResult;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.TestExecutionResult;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.support.descriptor.EngineDescriptor;
import org.junit.platform.fakes.TestEngineStub;

abstract class AbstractLauncherDiscoveryListenerTests {
public class FaultyTestEngines {

protected TestEngineStub createEngineThatCannotResolveAnything(String engineId) {
public static TestEngineStub createEngineThatCannotResolveAnything(String engineId) {
return new TestEngineStub(engineId) {
@Override
public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId uniqueId) {
Expand All @@ -29,10 +30,18 @@ public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId
selector, SelectorResolutionResult.unresolved()));
return new EngineDescriptor(uniqueId, "Some Engine");
}

@Override
public void execute(ExecutionRequest request) {
var listener = request.getEngineExecutionListener();
var rootTestDescriptor = request.getRootTestDescriptor();
listener.executionStarted(rootTestDescriptor);
listener.executionFinished(rootTestDescriptor, TestExecutionResult.successful());
}
};
}

protected TestEngineStub createEngineThatFailsToResolveAnything(String engineId, RuntimeException rootCause) {
public static TestEngineStub createEngineThatFailsToResolveAnything(String engineId, Throwable rootCause) {
return new TestEngineStub(engineId) {
@Override
public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId uniqueId) {
Expand All @@ -41,7 +50,14 @@ public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId
selector, SelectorResolutionResult.failed(rootCause)));
return new EngineDescriptor(uniqueId, "Some Engine");
}

@Override
public void execute(ExecutionRequest request) {
var listener = request.getEngineExecutionListener();
var rootTestDescriptor = request.getRootTestDescriptor();
listener.executionStarted(rootTestDescriptor);
listener.executionFinished(rootTestDescriptor, TestExecutionResult.successful());
}
};
}

}
21 changes: 21 additions & 0 deletions ...form-launcher/src/main/java/org/junit/platform/launcher/core/DiscoveryIssueCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@

package org.junit.platform.launcher.core;

import static org.junit.platform.engine.SelectorResolutionResult.Status.FAILED;
import static org.junit.platform.engine.SelectorResolutionResult.Status.UNRESOLVED;

import java.util.ArrayList;
import java.util.List;

import org.junit.platform.engine.DiscoveryIssue;
import org.junit.platform.engine.DiscoveryIssue.Severity;
import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.SelectorResolutionResult;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.discovery.UniqueIdSelector;
import org.junit.platform.launcher.LauncherDiscoveryListener;

class DiscoveryIssueCollector implements LauncherDiscoveryListener {
Expand All @@ -27,6 +33,21 @@ public void engineDiscoveryStarted(UniqueId engineId) {
this.issues.clear();
}

@Override
public void selectorProcessed(UniqueId engineId, DiscoverySelector selector, SelectorResolutionResult result) {
if (result.getStatus() == FAILED) {
this.issues.add(DiscoveryIssue.builder(Severity.ERROR, selector + " resolution failed") //
.cause(result.getThrowable()) //
.build());
}
else if (result.getStatus() == UNRESOLVED && selector instanceof UniqueIdSelector) {
UniqueId uniqueId = ((UniqueIdSelector) selector).getUniqueId();
if (uniqueId.hasPrefix(engineId)) {
this.issues.add(DiscoveryIssue.create(Severity.ERROR, selector + " could not be resolved"));
}
}
}

@Override
public void issueEncountered(UniqueId engineId, DiscoveryIssue issue) {
this.issues.add(issue);
Expand Down
20 changes: 0 additions & 20 deletions .../junit/platform/launcher/listeners/discovery/AbortOnFailureLauncherDiscoveryListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,8 @@

package org.junit.platform.launcher.listeners.discovery;

import static org.junit.platform.engine.SelectorResolutionResult.Status.FAILED;
import static org.junit.platform.engine.SelectorResolutionResult.Status.UNRESOLVED;

import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.util.ExceptionUtils;
import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.SelectorResolutionResult;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.discovery.UniqueIdSelector;
import org.junit.platform.launcher.EngineDiscoveryResult;
import org.junit.platform.launcher.LauncherDiscoveryListener;

Expand All @@ -33,19 +26,6 @@ public void engineDiscoveryFinished(UniqueId engineId, EngineDiscoveryResult res
result.getThrowable().ifPresent(ExceptionUtils::throwAsUncheckedException);
}

@Override
public void selectorProcessed(UniqueId engineId, DiscoverySelector selector, SelectorResolutionResult result) {
if (result.getStatus() == FAILED) {
throw new JUnitException(selector + " resolution failed", result.getThrowable().orElse(null));
}
if (result.getStatus() == UNRESOLVED && selector instanceof UniqueIdSelector) {
UniqueId uniqueId = ((UniqueIdSelector) selector).getUniqueId();
if (uniqueId.hasPrefix(engineId)) {
throw new JUnitException(selector + " could not be resolved");
}
}
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
Expand Down
18 changes: 8 additions & 10 deletions jupiter-tests/src/test/java/org/junit/jupiter/engine/NestedTestClassesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@

package org.junit.jupiter.engine;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.util.Throwables.getRootCause;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
import static org.junit.platform.testkit.engine.EventConditions.finishedWithFailure;
import static org.junit.platform.testkit.engine.TestExecutionResultConditions.message;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
Expand All @@ -28,7 +27,6 @@
import org.junit.jupiter.engine.NestedTestClassesTests.OuterClass.NestedClass;
import org.junit.jupiter.engine.NestedTestClassesTests.OuterClass.NestedClass.RecursiveNestedClass;
import org.junit.jupiter.engine.NestedTestClassesTests.OuterClass.NestedClass.RecursiveNestedSiblingClass;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.launcher.LauncherDiscoveryRequest;
import org.junit.platform.testkit.engine.EngineExecutionResults;
Expand Down Expand Up @@ -180,12 +178,12 @@ void individualMethodsWithinRecursiveNestedTestClassHierarchiesAreExecuted() {
}

private void assertNestedCycle(Class<?> start, Class<?> from, Class<?> to) {
assertThatExceptionOfType(JUnitException.class)//
.isThrownBy(() -> executeTestsForClass(start))//
.withCauseExactlyInstanceOf(JUnitException.class)//
.satisfies(ex -> assertThat(getRootCause(ex)).hasMessageMatching(
String.format("Detected cycle in inner class hierarchy between .+%s and .+%s", from.getSimpleName(),
to.getSimpleName())));
var results = executeTestsForClass(start);
var expectedMessage = String.format(
"Cause: org.junit.platform.commons.JUnitException: Detected cycle in inner class hierarchy between %s and %s",
from.getName(), to.getName());
results.containerEvents().assertThatEvents() //
.haveExactly(1, finishedWithFailure(message(it -> it.contains(expectedMessage))));
}

// -------------------------------------------------------------------
Expand Down
51 changes: 48 additions & 3 deletions platform-tests/src/test/java/org/junit/platform/launcher/core/DefaultLauncherTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package org.junit.platform.launcher.core;

import static java.util.Objects.requireNonNull;
import static java.util.function.UnaryOperator.identity;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -19,8 +20,11 @@
import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;
import static org.junit.platform.engine.SelectorResolutionResult.unresolved;
import static org.junit.platform.engine.TestExecutionResult.successful;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectPackage;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
import static org.junit.platform.fakes.FaultyTestEngines.createEngineThatCannotResolveAnything;
import static org.junit.platform.fakes.FaultyTestEngines.createEngineThatFailsToResolveAnything;
import static org.junit.platform.launcher.LauncherConstants.DRY_RUN_PROPERTY_NAME;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.DEFAULT_DISCOVERY_LISTENER_CONFIGURATION_PROPERTY_NAME;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
Expand All @@ -38,6 +42,7 @@
import java.time.Instant;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.UnaryOperator;
import java.util.logging.Level;
import java.util.logging.LogRecord;

Expand Down Expand Up @@ -844,7 +849,47 @@ public void execute(ExecutionRequest request) {
.isBetween(result.startTime(), result.finishTime());
}

@Test
void reportsEngineExecutionFailureOnUnresolvedUniqueIdSelectorWithEnginePrefix() {
var engine = createEngineThatCannotResolveAnything("some-engine");
var selector = selectUniqueId(UniqueId.forEngine(engine.getId()));
var result = execute(engine, request -> request.selectors(selector));

assertThat(result.testExecutionResult().getStatus()).isEqualTo(Status.FAILED);
assertThat(result.testExecutionResult().getThrowable().orElseThrow()) //
.hasMessageStartingWith(
"TestEngine with ID 'some-engine' encountered a critical issue during test discovery") //
.hasMessageContaining("(1) [ERROR] %s could not be resolved", selector);
}

@Test
void ignoresUnresolvedUniqueIdSelectorWithoutEnginePrefix() {
var engine = createEngineThatCannotResolveAnything("some-engine");
var selector = selectUniqueId(UniqueId.forEngine("some-other-engine"));
var result = execute(engine, request -> request.selectors(selector));

assertThat(result.testExecutionResult().getStatus()).isEqualTo(Status.SUCCESSFUL);
}

@Test
void reportsEngineExecutionFailureForSelectorResolutionFailure() {
var engine = createEngineThatFailsToResolveAnything("some-engine", new RuntimeException("boom"));
var selector = selectClass(Object.class);
var result = execute(engine, request -> request.selectors(selector));

assertThat(result.testExecutionResult().getStatus()).isEqualTo(Status.FAILED);
assertThat(result.testExecutionResult().getThrowable().orElseThrow()) //
.hasMessageStartingWith(
"TestEngine with ID 'some-engine' encountered a critical issue during test discovery") //
.hasMessageContaining("(1) [ERROR] %s resolution failed", selector) //
.hasMessageContaining("Cause: java.lang.RuntimeException: boom");
}

private static ReportedData execute(TestEngine engine) {
return execute(engine, identity());
}

private static ReportedData execute(TestEngine engine, UnaryOperator<LauncherDiscoveryRequestBuilder> configurer) {
var executionListener = mock(TestExecutionListener.class);

AtomicReference<Instant> startTime = new AtomicReference<>();
Expand All @@ -859,9 +904,9 @@ private static ReportedData execute(TestEngine engine) {
return null;
}).when(executionListener).executionFinished(any(), any());

var request = request() //
.configurationParameter(DEFAULT_DISCOVERY_LISTENER_CONFIGURATION_PROPERTY_NAME, "logging") //
.build();
var builder = request() //
.configurationParameter(DEFAULT_DISCOVERY_LISTENER_CONFIGURATION_PROPERTY_NAME, "logging");
var request = configurer.apply(builder).build();
var launcher = createLauncher(engine);

var testPlan = launcher.discover(request);
Expand Down
48 changes: 1 addition & 47 deletions ...t/platform/launcher/listeners/discovery/AbortOnFailureLauncherDiscoveryListenerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
package org.junit.platform.launcher.listeners.discovery;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
import static org.junit.platform.launcher.core.LauncherFactoryForTestingPurposesOnly.createLauncher;
Expand All @@ -26,51 +24,7 @@
import org.junit.platform.engine.UniqueId;
import org.junit.platform.fakes.TestEngineStub;

class AbortOnFailureLauncherDiscoveryListenerTests extends AbstractLauncherDiscoveryListenerTests {

@Test
void abortsDiscoveryOnUnresolvedUniqueIdSelectorWithEnginePrefix() {
var engine = createEngineThatCannotResolveAnything("some-engine");
var request = request() //
.listeners(abortOnFailure()) //
.selectors(selectUniqueId(UniqueId.forEngine(engine.getId()))) //
.build();
var launcher = createLauncher(engine);

var exception = assertThrows(JUnitException.class, () -> launcher.discover(request));
assertThat(exception).hasMessage("TestEngine with ID 'some-engine' failed to discover tests");
assertThat(exception.getCause()).hasMessage(
"UniqueIdSelector [uniqueId = [engine:some-engine]] could not be resolved");
}

@Test
void doesNotAbortDiscoveryOnUnresolvedUniqueIdSelectorWithoutEnginePrefix() {
var engine = createEngineThatCannotResolveAnything("some-engine");
var request = request() //
.listeners(abortOnFailure()) //
.selectors(selectUniqueId(UniqueId.forEngine("some-other-engine"))) //
.build();
var launcher = createLauncher(engine);

assertDoesNotThrow(() -> launcher.discover(request));
}

@Test
void abortsDiscoveryOnSelectorResolutionFailure() {
var rootCause = new RuntimeException();
var engine = createEngineThatFailsToResolveAnything("some-engine", rootCause);
var request = request() //
.listeners(abortOnFailure()) //
.selectors(selectClass(Object.class)) //
.build();
var launcher = createLauncher(engine);

var exception = assertThrows(JUnitException.class, () -> launcher.discover(request));
assertThat(exception).hasMessage("TestEngine with ID 'some-engine' failed to discover tests");
assertThat(exception.getCause()) //
.hasMessageEndingWith("resolution failed") //
.cause().isSameAs(rootCause);
}
class AbortOnFailureLauncherDiscoveryListenerTests {

@Test
void abortsDiscoveryOnEngineDiscoveryFailure() {
Expand Down
4 changes: 3 additions & 1 deletion ...rg/junit/platform/launcher/listeners/discovery/LoggingLauncherDiscoveryListenerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
import static org.junit.platform.fakes.FaultyTestEngines.createEngineThatCannotResolveAnything;
import static org.junit.platform.fakes.FaultyTestEngines.createEngineThatFailsToResolveAnything;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.DEFAULT_DISCOVERY_LISTENER_CONFIGURATION_PROPERTY_NAME;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
import static org.junit.platform.launcher.core.LauncherFactoryForTestingPurposesOnly.createLauncher;
Expand All @@ -29,7 +31,7 @@
import org.junit.platform.fakes.TestEngineStub;

@TrackLogRecords
public class LoggingLauncherDiscoveryListenerTests extends AbstractLauncherDiscoveryListenerTests {
public class LoggingLauncherDiscoveryListenerTests {

@Test
void logsWarningOnUnresolvedUniqueIdSelectorWithEnginePrefix(LogRecordListener log) {
Expand Down
Loading