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

Add parseable DiscoverySelector representations #3737

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

leonard84
Copy link
Collaborator

@leonard84 leonard84 commented Mar 18, 2024

Overview

Before this PR, it was not possible to use the more advanced DiscoverySelectors without writing code. Now, we have a generic way to specify all supported DiscoverySelectors as strings and parse them.
This will allow advanced usage scenarios. For example, Gradle can add a Test task parameter to pass a custom selector directly to the platform. This would allow using the IterationSelector when delegating to Gradle in IDEA.


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

to programmatically create valid selector strings,
by creating a selector via DiscoverySelectors.select* and then calling .toSelectorString()
to get the serialized version of it.
Previous to this commit, only a minimal subset of characters has been escaped,
thus leaving the UniqueId unsafe for use in an url.
With the addition of a string representation of selectors based on the URI
format, it has become necessary to make UniqueId url-safe.
Instead of double-encoding the UniqueId, we make the default `toString()` form URL safe.

Still some problems regarding the limits of encoding/decoding
@marcphilipp marcphilipp force-pushed the leo/discovery-selectors-parse branch from 2e8b766 to 252f02f Compare March 22, 2024 15:12
@leonard84
Copy link
Collaborator Author

@marcphilipp, it looks good, what is missing? Or, are you waiting for me to switch it from draft to rfr?

@marcphilipp
Copy link
Member

@leonard84 Yes, it does look good and I will finish it for 5.11 M2.

@leonard84 leonard84 marked this pull request as ready for review May 1, 2024 12:17
*/
@API(status = EXPERIMENTAL, since = "1.11")
public static Optional<? extends DiscoverySelector> parse(String identifier) {
return new DiscoverySelectorIdentifierParsers().parse(identifier);
Copy link
Member

Choose a reason for hiding this comment

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

Creating a new instance of DiscoverySelectorIdentifierParsers here which leads to a new ServiceLoader lookup and instantiation does not feel right here. I don't think we should make it static, though. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants