From 492cbb5b2d2a0a9b64f328228d8f033eb99dca6d Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sat, 22 Nov 2025 12:28:23 +0100 Subject: [PATCH 1/7] Document test and container are non-exclusive properties * Define test and container as non-exclusive properties. * Use emphasis to distinguish between term and word. * Document that TestDescriptor.Type values are exclusive properties. * Document that `getChildren()`, `getDescendants()` and `mayRegisterTests()` must be consistent with `isContainer()`. --- .../junit/platform/engine/TestDescriptor.java | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java index 8d146c05b616..6a6b44134282 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java @@ -27,8 +27,8 @@ import org.junit.platform.commons.util.Preconditions; /** - * Mutable descriptor for a test or container that has been discovered by a - * {@link TestEngine}. + * Mutable descriptor for a test or container that has + * been discovered by a {@link TestEngine}. * * @since 1.0 * @see TestEngine @@ -106,6 +106,9 @@ default String getLegacyReportingName() { /** * Get the immutable set of children of this descriptor. * + *

The implementation must be consistent with {@link #isContainer()} such that + * {@code !x.container()} implies {@code x.getChildren().isEmpty()}. + * * @return the set of children of this descriptor; neither {@code null} * nor mutable, but potentially empty * @see #getDescendants() @@ -141,6 +144,9 @@ default Set getAncestors() { *

A descendant is a child of this descriptor or a child of one of * its children, recursively. * + *

The implementation must be consistent with {@link #isContainer()} such that + * {@code !x.container()} implies {@code x.getDescendants().isEmpty()}. + * * @see #getChildren() */ default Set getDescendants() { @@ -230,7 +236,11 @@ default boolean isRoot() { Type getType(); /** - * Determine if this descriptor describes a container. + * Determine if this descriptor describes a container. + * + *

A test descriptor is a container when it may contain other + * containers or tests as its children. A container can also be a + * test. * *

The default implementation delegates to {@link Type#isContainer()}. */ @@ -239,7 +249,11 @@ default boolean isContainer() { } /** - * Determine if this descriptor describes a test. + * Determine if this descriptor describes a test. + * + *

A test descriptor is a test when it verifies expected + * behavior when executed. A test can also be a + * container. * *

The default implementation delegates to {@link Type#isTest()}. */ @@ -250,6 +264,9 @@ default boolean isTest() { /** * Determine if this descriptor may register dynamic tests during execution. * + *

The implementation must be consistent with {@link #isContainer()} such that + * {@code !x.container()} implies {@code !x.mayRegisterTests()}. + * *

The default implementation assumes tests are usually known during * discovery and thus returns {@code false}. */ @@ -259,7 +276,7 @@ default boolean mayRegisterTests() { /** * Determine if the supplied descriptor (or any of its descendants) - * {@linkplain TestDescriptor#isTest() is a test} or + * {@linkplain TestDescriptor#isTest() is a test} or * {@linkplain TestDescriptor#mayRegisterTests() may potentially register * tests dynamically}. * @@ -355,12 +372,14 @@ static Visitor composite(Visitor... visitors) { enum Type { /** - * Denotes that the {@link TestDescriptor} is for a container. + * Denotes that the {@link TestDescriptor} is strictly for a + * container. I.e. it is not also a test. */ CONTAINER, /** - * Denotes that the {@link TestDescriptor} is for a test. + * Denotes that the {@link TestDescriptor} is strictly for a + * test. I.e. it is not also a container. */ TEST, From 365ad026f99bf42b3a09442ebc5defe844292871 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 28 Nov 2025 13:42:13 +0100 Subject: [PATCH 2/7] Require isContainer and isTest to be consistent with getType --- .../junit/platform/engine/TestDescriptor.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java index 6a6b44134282..a5f7d265411a 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java @@ -239,9 +239,12 @@ default boolean isRoot() { * Determine if this descriptor describes a container. * *

A test descriptor is a container when it may contain other - * containers or tests as its children. A container can also be a - * test. + * containers or tests as its children. In addition to being a + * container this test descriptor may also be a test. * + *

The implementation must be consistent with {@link #getType()} such that + * {@code x.isContainer()} equals {@code x.getType().isContainer()}. + * *

The default implementation delegates to {@link Type#isContainer()}. */ default boolean isContainer() { @@ -252,9 +255,12 @@ default boolean isContainer() { * Determine if this descriptor describes a test. * *

A test descriptor is a test when it verifies expected - * behavior when executed. A test can also be a - * container. - * + * behavior when executed. In addition to being a test this + * test descriptor may also be a container. + * + *

The implementation must be consistent with {@link #getType()} such that + * {@code x.isTest()} equals {@code x.getType().isTest()}. + * *

The default implementation delegates to {@link Type#isTest()}. */ default boolean isTest() { From ee1252a3208ce0e35959da28cecb6862061253f8 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 28 Nov 2025 14:00:37 +0100 Subject: [PATCH 3/7] Clarify that the only hierarchy is intended to be mutable --- .../junit/platform/engine/TestDescriptor.java | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java index a5f7d265411a..b211981a3447 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java @@ -27,9 +27,9 @@ import org.junit.platform.commons.util.Preconditions; /** - * Mutable descriptor for a test or container that has - * been discovered by a {@link TestEngine}. - * + * A descriptor with a mutable hierarchy for a test or container that has been + * discovered by a {@link TestEngine}. + * * @since 1.0 * @see TestEngine */ @@ -41,7 +41,10 @@ public interface TestDescriptor { * *

Uniqueness must be guaranteed across an entire test plan, * regardless of how many engines are used behind the scenes. - * + * + *

Implementations must treat this property as immutable after test + * discovery has completed. + * * @return the {@code UniqueId} for this descriptor; never {@code null} */ UniqueId getUniqueId(); @@ -76,7 +79,10 @@ default String getLegacyReportingName() { /** * Get the set of {@linkplain TestTag tags} associated with this descriptor. - * + * + *

Implementations must treat this property as immutable after test + * discovery has completed. + * * @return the set of tags associated with this descriptor; never {@code null} * but potentially empty * @see TestTag @@ -87,6 +93,9 @@ default String getLegacyReportingName() { * Get the {@linkplain TestSource source} of the test or container described * by this descriptor, if available. * + *

Implementations must treat this property as immutable after test + * discovery has completed. + * * @see TestSource */ Optional getSource(); @@ -229,6 +238,9 @@ default boolean isRoot() { /** * Determine the {@link Type} of this descriptor. * + *

Implementations must treat this property as immutable after test + * discovery has completed. + * * @return the descriptor type; never {@code null}. * @see #isContainer() * @see #isTest() @@ -273,6 +285,9 @@ default boolean isTest() { *

The implementation must be consistent with {@link #isContainer()} such that * {@code !x.container()} implies {@code !x.mayRegisterTests()}. * + *

Implementations must treat this property as immutable after test + * discovery has completed. + * *

The default implementation assumes tests are usually known during * discovery and thus returns {@code false}. */ From 3481b2ff27d5ad4ac056964bb925f0ea7b284c17 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 28 Nov 2025 14:08:31 +0100 Subject: [PATCH 4/7] Spotless --- .../junit/platform/engine/TestDescriptor.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java index b211981a3447..68f1ddbd53f5 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java @@ -29,7 +29,7 @@ /** * A descriptor with a mutable hierarchy for a test or container that has been * discovered by a {@link TestEngine}. - * + * * @since 1.0 * @see TestEngine */ @@ -41,10 +41,10 @@ public interface TestDescriptor { * *

Uniqueness must be guaranteed across an entire test plan, * regardless of how many engines are used behind the scenes. - * + * *

Implementations must treat this property as immutable after test * discovery has completed. - * + * * @return the {@code UniqueId} for this descriptor; never {@code null} */ UniqueId getUniqueId(); @@ -79,10 +79,10 @@ default String getLegacyReportingName() { /** * Get the set of {@linkplain TestTag tags} associated with this descriptor. - * + * *

Implementations must treat this property as immutable after test * discovery has completed. - * + * * @return the set of tags associated with this descriptor; never {@code null} * but potentially empty * @see TestTag @@ -240,7 +240,7 @@ default boolean isRoot() { * *

Implementations must treat this property as immutable after test * discovery has completed. - * + * * @return the descriptor type; never {@code null}. * @see #isContainer() * @see #isTest() @@ -256,7 +256,7 @@ default boolean isRoot() { * *

The implementation must be consistent with {@link #getType()} such that * {@code x.isContainer()} equals {@code x.getType().isContainer()}. - * + * *

The default implementation delegates to {@link Type#isContainer()}. */ default boolean isContainer() { @@ -269,10 +269,10 @@ default boolean isContainer() { *

A test descriptor is a test when it verifies expected * behavior when executed. In addition to being a test this * test descriptor may also be a container. - * + * *

The implementation must be consistent with {@link #getType()} such that * {@code x.isTest()} equals {@code x.getType().isTest()}. - * + * *

The default implementation delegates to {@link Type#isTest()}. */ default boolean isTest() { @@ -287,7 +287,7 @@ default boolean isTest() { * *

Implementations must treat this property as immutable after test * discovery has completed. - * + * *

The default implementation assumes tests are usually known during * discovery and thus returns {@code false}. */ From c2702d96723feaccdb845a43d92c064a6d73a9a0 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 28 Nov 2025 14:24:17 +0100 Subject: [PATCH 5/7] Use consistent language --- .../junit/platform/engine/TestDescriptor.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java index 68f1ddbd53f5..0dc47bca49cc 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java @@ -42,7 +42,7 @@ public interface TestDescriptor { *

Uniqueness must be guaranteed across an entire test plan, * regardless of how many engines are used behind the scenes. * - *

Implementations must treat this property as immutable after test + *

The implementation must treat this property as immutable after test * discovery has completed. * * @return the {@code UniqueId} for this descriptor; never {@code null} @@ -80,7 +80,7 @@ default String getLegacyReportingName() { /** * Get the set of {@linkplain TestTag tags} associated with this descriptor. * - *

Implementations must treat this property as immutable after test + *

The implementation must treat this property as immutable after test * discovery has completed. * * @return the set of tags associated with this descriptor; never {@code null} @@ -93,7 +93,7 @@ default String getLegacyReportingName() { * Get the {@linkplain TestSource source} of the test or container described * by this descriptor, if available. * - *

Implementations must treat this property as immutable after test + *

The implementation must treat this property as immutable after test * discovery has completed. * * @see TestSource @@ -238,7 +238,7 @@ default boolean isRoot() { /** * Determine the {@link Type} of this descriptor. * - *

Implementations must treat this property as immutable after test + *

The implementation must treat this property as immutable after test * discovery has completed. * * @return the descriptor type; never {@code null}. @@ -254,8 +254,8 @@ default boolean isRoot() { * containers or tests as its children. In addition to being a * container this test descriptor may also be a test. * - *

The implementation must be consistent with {@link #getType()} such that - * {@code x.isContainer()} equals {@code x.getType().isContainer()}. + *

The implementation must be consistent with {@link #getType()} such + * that {@code x.isContainer()} equals {@code x.getType().isContainer()}. * *

The default implementation delegates to {@link Type#isContainer()}. */ @@ -270,8 +270,8 @@ default boolean isContainer() { * behavior when executed. In addition to being a test this * test descriptor may also be a container. * - *

The implementation must be consistent with {@link #getType()} such that - * {@code x.isTest()} equals {@code x.getType().isTest()}. + *

The implementation must be consistent with {@link #getType()} such + * that {@code x.isTest()} equals {@code x.getType().isTest()}. * *

The default implementation delegates to {@link Type#isTest()}. */ @@ -282,11 +282,9 @@ default boolean isTest() { /** * Determine if this descriptor may register dynamic tests during execution. * - *

The implementation must be consistent with {@link #isContainer()} such that - * {@code !x.container()} implies {@code !x.mayRegisterTests()}. - * - *

Implementations must treat this property as immutable after test - * discovery has completed. + *

The implementation must treat this property as immutable after test + * discovery has completed and must be consistent with {@link #isContainer()} + * such that {@code !x.container()} implies {@code !x.mayRegisterTests()}. * *

The default implementation assumes tests are usually known during * discovery and thus returns {@code false}. From 41fac5d2816de579971857a3d2eef2f130ac2026 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 28 Nov 2025 14:47:44 +0100 Subject: [PATCH 6/7] Update release notes --- .../src/docs/asciidoc/release-notes/release-notes-6.1.0-M2.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M2.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M2.adoc index 4f40bc523b81..4def1151ca40 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M2.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M2.adoc @@ -16,7 +16,7 @@ repository on GitHub. [[release-notes-6.1.0-M2-junit-platform-bug-fixes]] ==== Bug Fixes -* ❓ +* Clarify `TestDescriptor` implementation requirements. [[release-notes-6.1.0-M2-junit-platform-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes From c05d54968eace817477f4f0a72846e013cd719f1 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 28 Nov 2025 14:52:38 +0100 Subject: [PATCH 7/7] Document immutable test descriptor requirement with mutable hierarchy --- .../src/docs/asciidoc/user-guide/advanced-topics/engines.adoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/documentation/src/docs/asciidoc/user-guide/advanced-topics/engines.adoc b/documentation/src/docs/asciidoc/user-guide/advanced-topics/engines.adoc index 03615613456f..8876443f0461 100644 --- a/documentation/src/docs/asciidoc/user-guide/advanced-topics/engines.adoc +++ b/documentation/src/docs/asciidoc/user-guide/advanced-topics/engines.adoc @@ -95,6 +95,8 @@ to the following requirements: * The `TestDescriptor` returned from `TestEngine.discover()` _must_ be the root of a tree of `TestDescriptor` instances. This implies that there _must not_ be any cycles between a node and its descendants. +* The hierarchy of test descriptors returned from `TestEngine.discover()` _must_ be + mutable, but the test descriptors _must_ otherwise be immutable after discovery. * A `TestEngine` _must_ be able to discover `UniqueIdSelectors` for any unique ID that it previously generated and returned from `TestEngine.discover()`. This enables selecting a subset of tests to execute or rerun.