7903930: Support running individual parameterized tests and @Nested test classes #241

JornVernee · 2025-01-13T13:19:55Z

See the JBS issue for an extended problem description.

This PR adds support for running individual parameterized JUnit test methods, as well as nested test classes, by adding a new flavor of query string, prefixed with junit-select:

junit-select:<selector>

This intends to take direct advantage of JUnits selector string support. The supported selectors are listed in the column on the right in the table here: https://junit.org/junit5/docs/current/user-guide/#running-tests-discovery-selectors

For example, running a method foo that takes no parameters could be done using foo, just like before. If foo has parameters, then for JUnit tests they have to be specified as a comma separated list of binary names. For that we can use the method selector. For example:

junit-select:method:TestDowncallScope#testDowncall(int,java.lang.String,CallGeneratorHelper$Ret,java.util.List,java.util.List)

This support is intended to be used by the intellij plugin to support running individual methods and nested classes (see #240).

Testing: I've added additional tests for the new cases. I've also been using a PoC of this feature as the basis for similar support in the intellij plugin that I'm working on.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

CODETOOLS-7903930: Support running individual parameterized tests and @Nested test classes (Enhancement - P4)

Reviewers

Christian Stein (@sormuras - Committer)
Jaikiran Pai (@jaikiran - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jtreg.git pull/241/head:pull/241
$ git checkout pull/241

Update a local copy of the PR:
$ git checkout pull/241
$ git pull https://git.openjdk.org/jtreg.git pull/241/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 241

View PR using the GUI difftool:
$ git pr show -t 241

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jtreg/pull/241.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-01-13T13:20:28Z

👋 Welcome back jvernee! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-01-13T13:21:20Z

@JornVernee This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

7903930: Support running individual parameterized tests and @Nested test classes

Reviewed-by: cstein, jpai

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

9bc320a: 7903883: --verify-exclude test existence and format checks are broken
8e74d55: 7903872: Option testThreadFactoryPath support relativepath

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

mlbridge · 2025-01-13T13:31:28Z

Webrevs

sormuras · 2025-01-14T15:40:36Z

Implementation and associated tests look good on first sight! Will do some local experiments tomorrow.

Please, also update related sections/phrases in the FAQ document, for example:

JornVernee · 2025-01-14T16:25:23Z

I've updated the FAQ. See: 54d91bd

…t classes" Revert TestNG related changes

JornVernee · 2025-01-14T18:06:31Z

Some results of offline discussion:

We'd like to rely on JUnit's builtin support for parsing these queries, so we don't have to implement our own parsing. This also means we can use any JUnit selector (such as uid, to run individual cases of a parameterized test)
This means that we don't support the extended format for TestNG tests. But, this is okay, since TestNG doesn't support nested test classes any way, and we don't need to specify the parameter types to select a parameterized method
To use the junit selector support, the query string is prefixed with junit-select:, and then an identifier such as the ones listed here is used as a suffix.
The current format, with just the method name, continues to work as well.

…s from group specs

sormuras

Thanks for implementing this feature, Jorn, with updates to the FAQ and tests.

Nit: please delete unused import statements and add a line to the CHANGELOG.md file

src/share/classes/com/sun/javatest/regtest/agent/JUnitRunner.java

sormuras · 2025-01-20T16:22:31Z

src/share/classes/com/sun/javatest/regtest/config/TestManager.java

+                ? Pattern.compile("(?<dir>|[^A-Za-z?]|[^?]{2,}):(?<group>[A-Za-z0-9_,]+)")
+                : Pattern.compile("(?<dir>[^?]*):(?<group>[A-Za-z0-9_,]+)");


Good improvement!

Co-authored-by: Christian Stein <[email protected]>

sbrannen · 2025-01-20T16:46:12Z

If foo has parameters, then for JUnit tests they have to be specified as a comma separated list of binary names.

For array types, I believe that canonical names are also supported -- for example, int[][], java.lang.String[], etc., which might prove beneficial if any of those queries are hand crafted.

JornVernee · 2025-01-20T16:54:31Z

If foo has parameters, then for JUnit tests they have to be specified as a comma separated list of binary names.

For array types, I believe that canonical names are also supported -- for example, int[][], java.lang.String[], etc., which might prove beneficial if any of those queries are hand crafted.

Yes, that's a good point. I think the main user of this will be the intellij plugin, which will automatically derive the query string, but it wouldn't hurt to mention this in the FAQ. I'll try to see if there's a JUnit doc section that describes the supported format, and we could just link to that.

JornVernee · 2025-01-20T17:04:43Z

I noticed that there's also a nested-class and nested-method selector. Is there a reason for keeping these separate? Or is it fine to use the class and method selectors as well, which work as long as you use the binary name of the nested class?

sormuras · 2025-01-20T17:16:15Z

I noticed that there's also a nested-class and nested-method selector. Is there a reason for keeping these separate? Or is it fine to use the class and method selectors as well, which work as long as you use the binary name of the nested class?

Binary names for classes and nested classes are fine. These two nested variants can be used for "lazy load" scenarios. See for example the API documentation of NestedMethodSelector

A DiscoverySelector that selects a nested Method or a combination of enclosing classes names, class name, method name, and parameter types so that TestEngines can discover tests or containers based on methods.
If a Java Method is provided, the selector will return that method and its method name, class name, enclosing classes names, and parameter types accordingly. If class names or method names are provided, this selector will only attempt to lazily load a class or method if getEnclosingClasses(), getNestedClass(), getMethod(), or getParameterTypes() is invoked.

JornVernee · 2025-01-20T17:54:21Z

I've updated the FAQ to link to the doc of DiscoverySelectors#selectMethod(java.lang.String)

sormuras

Looks good to me.

jaikiran

Hello Jorn, these changes look reasonable to me.

Several of these files will need a copyright year update before integrating.

JornVernee · 2025-01-21T12:29:57Z

/integrate

openjdk · 2025-01-21T12:30:46Z

Going to push as commit b443973.
Since your change was applied there have been 2 commits pushed to the master branch:

9bc320a: 7903883: --verify-exclude test existence and format checks are broken
8e74d55: 7903872: Option testThreadFactoryPath support relativepath

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-21T12:30:52Z

@JornVernee Pushed as commit b443973.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Support running individual parameterized methods & nested test classes

6149386

JornVernee marked this pull request as ready for review January 13, 2025 13:24

openjdk bot added the rfr Pull request is ready for review label Jan 13, 2025

update FAQ

54d91bd

JornVernee added 4 commits January 14, 2025 17:03

Revert "Support running individual parameterized methods & nested tes…

7a68115

…t classes" Revert TestNG related changes

Rely on junit selector parsing support

d322a2c

cleanup test

9fec36f

update FAQ

e5c3728

JornVernee added 2 commits January 17, 2025 16:15

exclude '?' from directory pattern, so as not to confuse query string…

914f030

…s from group specs

add test case for prior fix

072b039

sormuras suggested changes Jan 20, 2025

View reviewed changes

Remove dangling imports

0ccb562

Co-authored-by: Christian Stein <[email protected]>

update changelog

d6be9d9

update FAQ to link to JUnit doc

310bf0f

fix typo

a90a014

sormuras approved these changes Jan 21, 2025

View reviewed changes

jaikiran approved these changes Jan 21, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 21, 2025

Update copyright years

a525947

sormuras approved these changes Jan 21, 2025

View reviewed changes

jaikiran approved these changes Jan 21, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jan 21, 2025

openjdk bot closed this Jan 21, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 21, 2025

JornVernee deleted the QStrings branch January 21, 2025 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7903930: Support running individual parameterized tests and @Nested test classes #241

7903930: Support running individual parameterized tests and @Nested test classes #241

JornVernee commented Jan 13, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 13, 2025

openjdk bot commented Jan 13, 2025 •

edited

Loading

mlbridge bot commented Jan 13, 2025 •

edited

Loading

sormuras commented Jan 14, 2025

JornVernee commented Jan 14, 2025

JornVernee commented Jan 14, 2025

sormuras left a comment

sormuras Jan 20, 2025

sbrannen commented Jan 20, 2025 •

edited

Loading

JornVernee commented Jan 20, 2025

JornVernee commented Jan 20, 2025 •

edited

Loading

sormuras commented Jan 20, 2025

JornVernee commented Jan 20, 2025

sormuras left a comment

jaikiran left a comment

JornVernee commented Jan 21, 2025

openjdk bot commented Jan 21, 2025

openjdk bot commented Jan 21, 2025

		? Pattern.compile("(?<dir>\|[^A-Za-z?]\|[^?]{2,}):(?<group>[A-Za-z0-9_,]+)")
		: Pattern.compile("(?<dir>[^?]*):(?<group>[A-Za-z0-9_,]+)");

7903930: Support running individual parameterized tests and @Nested test classes #241

7903930: Support running individual parameterized tests and @Nested test classes #241

Conversation

JornVernee commented Jan 13, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 13, 2025

openjdk bot commented Jan 13, 2025 • edited Loading

mlbridge bot commented Jan 13, 2025 • edited Loading

Webrevs

sormuras commented Jan 14, 2025

JornVernee commented Jan 14, 2025

JornVernee commented Jan 14, 2025

sormuras left a comment

Choose a reason for hiding this comment

sormuras Jan 20, 2025

Choose a reason for hiding this comment

sbrannen commented Jan 20, 2025 • edited Loading

JornVernee commented Jan 20, 2025

JornVernee commented Jan 20, 2025 • edited Loading

sormuras commented Jan 20, 2025

JornVernee commented Jan 20, 2025

sormuras left a comment

Choose a reason for hiding this comment

jaikiran left a comment

Choose a reason for hiding this comment

JornVernee commented Jan 21, 2025

openjdk bot commented Jan 21, 2025

openjdk bot commented Jan 21, 2025

JornVernee commented Jan 13, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 13, 2025 •

edited

Loading

mlbridge bot commented Jan 13, 2025 •

edited

Loading

sbrannen commented Jan 20, 2025 •

edited

Loading

JornVernee commented Jan 20, 2025 •

edited

Loading