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.

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

Avoid unnecessarily parsing unique IDs #3073

Merged
merged 2 commits into from
Oct 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions documentation/src/docs/asciidoc/release-notes/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ include::{includedir}/link-attributes.adoc[]

include::{basedir}/release-notes-5.10.0-M1.adoc[]

include::{basedir}/release-notes-5.9.2.adoc[]

include::{basedir}/release-notes-5.9.1.adoc[]

include::{basedir}/release-notes-5.9.0.adoc[]
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
[[release-notes-5.9.2]]
== 5.9.2

*Date of Release:* ❓

*Scope:* Bug fixes and enhancements since 5.9.1

For a complete list of all _closed_ issues and pull requests for this release, consult the
link:{junit5-repo}+/milestones/5.9.2+[5.9.2] milestone page in the
JUnit repository on GitHub.


[[release-notes-5.9.2-junit-platform]]
=== JUnit Platform

==== Bug Fixes

* ❓

==== Deprecations and Breaking Changes

* ❓

==== New Features and Improvements

* Introduce `TestPlan.getTestIdentifier(UniqueId)` and `TestPlan.getChildren(UniqueId)` to
avoid parsing unique IDs unnecessarily during test execution.


[[release-notes-5.9.2-junit-jupiter]]
=== JUnit Jupiter

==== Bug Fixes

* ❓

==== Deprecations and Breaking Changes

* ❓

==== New Features and Improvements

* ❓


= [[release-notes-5.9.2-junit-vintage]]
=== JUnit Vintage

==== Bug Fixes

* ❓

==== Deprecations and Breaking Changes

* ❓

==== New Features and Improvements

* ❓
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.launcher.TestIdentifier;
import org.junit.platform.testkit.engine.Event;
import org.junit.platform.testkit.engine.Events;
Expand All @@ -40,7 +41,7 @@ void executeTestCaseWithOverloadedMethodsAndThenRerunOnlyOneOfTheMethodsSelected
event -> event.getTestDescriptor().getUniqueId().toString().contains(TestInfo.class.getName())).findFirst();
assertTrue(first.isPresent());
TestIdentifier testIdentifier = TestIdentifier.from(first.get().getTestDescriptor());
String uniqueId = testIdentifier.getUniqueId();
UniqueId uniqueId = testIdentifier.getUniqueIdObject();

tests = executeTests(selectUniqueId(uniqueId)).testEvents();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.junit.platform.console.options.Theme;
import org.junit.platform.engine.TestExecutionResult;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.reporting.ReportEntry;
import org.junit.platform.launcher.TestExecutionListener;
import org.junit.platform.launcher.TestIdentifier;
Expand All @@ -27,23 +28,22 @@
*/
class TreePrintingListener implements TestExecutionListener {

private final Map<String, TreeNode> nodesByUniqueId = new ConcurrentHashMap<>();
private final Map<UniqueId, TreeNode> nodesByUniqueId = new ConcurrentHashMap<>();
private TreeNode root;
private final TreePrinter treePrinter;

TreePrintingListener(PrintWriter out, ColorPalette colorPalette, Theme theme) {
this.treePrinter = new TreePrinter(out, theme, colorPalette);
}

private TreeNode addNode(TestIdentifier testIdentifier, Supplier<TreeNode> nodeSupplier) {
private void addNode(TestIdentifier testIdentifier, Supplier<TreeNode> nodeSupplier) {
TreeNode node = nodeSupplier.get();
nodesByUniqueId.put(testIdentifier.getUniqueId(), node);
testIdentifier.getParentId().map(nodesByUniqueId::get).orElse(root).addChild(node);
return node;
nodesByUniqueId.put(testIdentifier.getUniqueIdObject(), node);
testIdentifier.getParentIdObject().map(nodesByUniqueId::get).orElse(root).addChild(node);
}

private TreeNode getNode(TestIdentifier testIdentifier) {
return nodesByUniqueId.get(testIdentifier.getUniqueId());
return nodesByUniqueId.get(testIdentifier.getUniqueIdObject());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
public class FlightRecordingExecutionListener implements TestExecutionListener {

private final AtomicReference<TestPlanExecutionEvent> testPlanExecutionEvent = new AtomicReference<>();
private final Map<String, TestExecutionEvent> testExecutionEvents = new ConcurrentHashMap<>();
private final Map<org.junit.platform.engine.UniqueId, TestExecutionEvent> testExecutionEvents = new ConcurrentHashMap<>();

@Override
public void testPlanExecutionStarted(TestPlan plan) {
Expand Down Expand Up @@ -70,15 +70,15 @@ public void executionSkipped(TestIdentifier test, String reason) {
@Override
public void executionStarted(TestIdentifier test) {
TestExecutionEvent event = new TestExecutionEvent();
testExecutionEvents.put(test.getUniqueId(), event);
testExecutionEvents.put(test.getUniqueIdObject(), event);
event.initialize(test);
event.begin();
}

@Override
public void executionFinished(TestIdentifier test, TestExecutionResult result) {
Optional<Throwable> throwable = result.getThrowable();
TestExecutionEvent event = testExecutionEvents.remove(test.getUniqueId());
TestExecutionEvent event = testExecutionEvents.remove(test.getUniqueIdObject());
event.end();
event.result = result.getStatus().toString();
event.exceptionClass = throwable.map(Throwable::getClass).orElse(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,19 +158,19 @@ public Set<TestIdentifier> getRoots() {
*/
public Optional<TestIdentifier> getParent(TestIdentifier child) {
Preconditions.notNull(child, "child must not be null");
return child.getParentId().map(this::getTestIdentifier);
return child.getParentIdObject().map(this::getTestIdentifier);
}

/**
* Get the children of the supplied {@link TestIdentifier}.
*
* @param parent the identifier to look up the children for; never {@code null}
* @return an unmodifiable set of the parent's children, potentially empty
* @see #getChildren(String)
* @see #getChildren(UniqueId)
*/
public Set<TestIdentifier> getChildren(TestIdentifier parent) {
Preconditions.notNull(parent, "parent must not be null");
return getChildren(parent.getUniqueId());
return getChildren(parent.getUniqueIdObject());
}

/**
Expand All @@ -180,11 +180,26 @@ public Set<TestIdentifier> getChildren(TestIdentifier parent) {
* {@code null} or blank
* @return an unmodifiable set of the parent's children, potentially empty
* @see #getChildren(TestIdentifier)
* @deprecated Use {@link #getChildren(UniqueId)}
*/
@API(status = DEPRECATED, since = "1.10")
@Deprecated
public Set<TestIdentifier> getChildren(String parentId) {
Preconditions.notBlank(parentId, "parent ID must not be null or blank");
UniqueId uniqueId = UniqueId.parse(parentId);
return children.containsKey(uniqueId) ? unmodifiableSet(children.get(uniqueId)) : emptySet();
return getChildren(UniqueId.parse(parentId));
}

/**
* Get the children of the supplied unique ID.
*
* @param parentId the unique ID to look up the children for; never
* {@code null}
* @return an unmodifiable set of the parent's children, potentially empty
* @see #getChildren(TestIdentifier)
*/
@API(status = MAINTAINED, since = "1.10")
public Set<TestIdentifier> getChildren(UniqueId parentId) {
return children.containsKey(parentId) ? unmodifiableSet(children.get(parentId)) : emptySet();
}

/**
Expand All @@ -195,13 +210,29 @@ public Set<TestIdentifier> getChildren(String parentId) {
* @return the identifier with the supplied unique ID; never {@code null}
* @throws PreconditionViolationException if no {@code TestIdentifier}
* with the supplied unique ID is present in this test plan
* @deprecated Use {@link #getTestIdentifier(UniqueId)}
*/
@API(status = DEPRECATED, since = "1.10")
@Deprecated
public TestIdentifier getTestIdentifier(String uniqueId) throws PreconditionViolationException {
Preconditions.notBlank(uniqueId, "unique ID must not be null or blank");
UniqueId uniqueIdObject = UniqueId.parse(uniqueId);
Preconditions.condition(allIdentifiers.containsKey(uniqueIdObject),
return getTestIdentifier(UniqueId.parse(uniqueId));
}

/**
* Get the {@link TestIdentifier} with the supplied unique ID.
*
* @param uniqueId the unique ID to look up the identifier for; never
* {@code null}
* @return the identifier with the supplied unique ID; never {@code null}
* @throws PreconditionViolationException if no {@code TestIdentifier}
* with the supplied unique ID is present in this test plan
*/
@API(status = MAINTAINED, since = "1.10")
public TestIdentifier getTestIdentifier(UniqueId uniqueId) {
marcphilipp marked this conversation as resolved.
Show resolved Hide resolved
Preconditions.notNull(uniqueId, () -> "uniqueId must not be null");
return Preconditions.notNull(allIdentifiers.get(uniqueId),
() -> "No TestIdentifier with unique ID [" + uniqueId + "] has been added to this TestPlan.");
return allIdentifiers.get(uniqueIdObject);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void reportingEntryPublished(TestDescriptor testDescriptor, ReportEntry e
}

private TestIdentifier getTestIdentifier(TestDescriptor testDescriptor) {
return this.testPlan.getTestIdentifier(testDescriptor.getUniqueId().toString());
return this.testPlan.getTestIdentifier(testDescriptor.getUniqueId());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.function.Predicate;

import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.launcher.TestIdentifier;
import org.junit.platform.launcher.TestPlan;

Expand Down Expand Up @@ -80,16 +81,28 @@ public Set<TestIdentifier> getChildren(TestIdentifier parent) {
return delegate.getChildren(parent);
}

@SuppressWarnings("deprecation")
@Override
public Set<TestIdentifier> getChildren(String parentId) {
return delegate.getChildren(parentId);
}

@Override
public Set<TestIdentifier> getChildren(UniqueId parentId) {
return delegate.getChildren(parentId);
}

@SuppressWarnings("deprecation")
@Override
public TestIdentifier getTestIdentifier(String uniqueId) throws PreconditionViolationException {
return delegate.getTestIdentifier(uniqueId);
}

@Override
public TestIdentifier getTestIdentifier(UniqueId uniqueId) {
return delegate.getTestIdentifier(uniqueId);
}

@Override
public long countTestIdentifiers(Predicate<? super TestIdentifier> predicate) {
return delegate.countTestIdentifiers(predicate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

import org.apiguardian.api.API;
import org.junit.platform.engine.TestExecutionResult;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.reporting.ReportEntry;
import org.junit.platform.launcher.TestExecutionListener;
import org.junit.platform.launcher.TestIdentifier;
Expand Down Expand Up @@ -107,7 +106,7 @@ public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult

private void writeXmlReportInCaseOfRoot(TestIdentifier testIdentifier) {
if (isRoot(testIdentifier)) {
String rootName = UniqueId.parse(testIdentifier.getUniqueId()).getSegments().get(0).getValue();
String rootName = testIdentifier.getUniqueIdObject().getSegments().get(0).getValue();
writeXmlReportSafely(testIdentifier, rootName);
}
}
Expand All @@ -123,7 +122,7 @@ private void writeXmlReportSafely(TestIdentifier testIdentifier, String rootName
}

private boolean isRoot(TestIdentifier testIdentifier) {
return !testIdentifier.getParentId().isPresent();
return !testIdentifier.getParentIdObject().isPresent();
}

private void printException(String message, Exception exception) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void filter(Filter filter) throws NoTestsRemainException {
private LauncherDiscoveryRequest createDiscoveryRequestForUniqueIds(Set<TestIdentifier> testIdentifiers) {
// @formatter:off
List<DiscoverySelector> selectors = testIdentifiers.stream()
.map(TestIdentifier::getUniqueId)
.map(TestIdentifier::getUniqueIdObject)
.map(DiscoverySelectors::selectUniqueId)
.collect(toList());
// @formatter:on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import org.junit.platform.engine.TestExecutionResult;
import org.junit.platform.engine.TestExecutionResult.Status;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.reporting.ReportEntry;
import org.junit.platform.launcher.TestExecutionListener;
import org.junit.platform.launcher.TestIdentifier;
Expand All @@ -37,7 +38,7 @@ class JUnitPlatformRunnerListener implements TestExecutionListener {

@Override
public void dynamicTestRegistered(TestIdentifier testIdentifier) {
String parentId = testIdentifier.getParentId().get();
UniqueId parentId = testIdentifier.getParentIdObject().get();
testTree.addDynamicDescription(testIdentifier, parentId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.platform.commons.util.AnnotationUtils;
import org.junit.platform.commons.util.StringUtils;
import org.junit.platform.engine.TestSource;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.support.descriptor.ClassSource;
import org.junit.platform.engine.support.descriptor.MethodSource;
import org.junit.platform.launcher.TestIdentifier;
Expand Down Expand Up @@ -86,7 +87,7 @@ private void buildDescriptionTree(Description suiteDescription, TestPlan testPla
testPlan.getRoots().forEach(testIdentifier -> buildDescription(testIdentifier, suiteDescription, testPlan));
}

void addDynamicDescription(TestIdentifier newIdentifier, String parentId) {
void addDynamicDescription(TestIdentifier newIdentifier, UniqueId parentId) {
Description parent = getDescription(this.testPlan.getTestIdentifier(parentId));
buildDescription(newIdentifier, parent, this.testPlan);
}
Expand All @@ -103,9 +104,9 @@ private Description createJUnit4Description(TestIdentifier identifier, TestPlan
String name = nameExtractor.apply(identifier);
if (identifier.isTest()) {
String containerName = testPlan.getParent(identifier).map(nameExtractor).orElse("<unrooted>");
return Description.createTestDescription(containerName, name, identifier.getUniqueId());
return Description.createTestDescription(containerName, name, identifier.getUniqueIdObject());
}
return Description.createSuiteDescription(name, identifier.getUniqueId());
return Description.createSuiteDescription(name, identifier.getUniqueIdObject());
}

private String getTechnicalName(TestIdentifier testIdentifier) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ void launcherWillNotExecuteEnginesIfNotIncludedByAnEngineFilter() {

assertThat(testPlan.getRoots()).hasSize(1);
var rootIdentifier = testPlan.getRoots().iterator().next();
assertThat(testPlan.getChildren(rootIdentifier.getUniqueId())).hasSize(1);
assertThat(testPlan.getChildren(UniqueId.forEngine("first").toString())).hasSize(1);
assertThat(testPlan.getChildren(rootIdentifier.getUniqueIdObject())).hasSize(1);
assertThat(testPlan.getChildren(UniqueId.forEngine("first"))).hasSize(1);
}

@Test
Expand Down Expand Up @@ -118,8 +118,8 @@ void launcherWillNotExecuteEnginesExplicitlyExcludedByAnEngineFilter() {

assertThat(testPlan.getRoots()).hasSize(1);
var rootIdentifier = testPlan.getRoots().iterator().next();
assertThat(testPlan.getChildren(rootIdentifier.getUniqueId())).hasSize(1);
assertThat(testPlan.getChildren(UniqueId.forEngine("first").toString())).hasSize(1);
assertThat(testPlan.getChildren(rootIdentifier.getUniqueIdObject())).hasSize(1);
assertThat(testPlan.getChildren(UniqueId.forEngine("first"))).hasSize(1);
}

@Test
Expand All @@ -143,8 +143,8 @@ void launcherWillExecuteEnginesHonoringBothIncludeAndExcludeEngineFilters() {

assertThat(testPlan.getRoots()).hasSize(1);
var rootIdentifier = testPlan.getRoots().iterator().next();
assertThat(testPlan.getChildren(rootIdentifier.getUniqueId())).hasSize(1);
assertThat(testPlan.getChildren(UniqueId.forEngine("first").toString())).hasSize(1);
assertThat(testPlan.getChildren(rootIdentifier.getUniqueIdObject())).hasSize(1);
assertThat(testPlan.getChildren(UniqueId.forEngine("first"))).hasSize(1);
}

@Test
Expand Down
Loading