-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Continuous Testing: add support for build system like test selection #46389
Continuous Testing: add support for build system like test selection #46389
Conversation
41fbfd8
to
285f414
Compare
This comment has been minimized.
This comment has been minimized.
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
I tried looking at the Istio tests and I honestly have no idea how they are supposed to work. The |
if (specificSelection.startsWith("maven:")) { | ||
launchBuilder.filters(new MavenSpecificSelectionFilter(specificSelection.substring("maven:".length()))); | ||
} else if (specificSelection.startsWith("gradle:")) { | ||
launchBuilder.filters(new GradleSpecificSelectionFilter(specificSelection.substring("gradle:".length()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering whether we could keep build tool-specific parsing somewhere in that build tool integration code (Gradle/Maven plugin impl) and pass a value in an expected format here.
That way we would keep build tool-specific code in the build tool integration module.
That raises the question which format we should adopt here: either our own version or pick one of Maven or Gradle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the Gradle format is super dumb and it could be translated to the Maven format ... if it didn't use a period .
as a separator of class name pattern and method name pattern. Figuring out what any given period means would require adding more heuristics to the parsing side which are currently not present (because the decision happens during evaluation).
Also, the place where we have access to the user-provided value is in a different process than the place where we have to apply it. So we have to "serialize" it and then "parse" again anyway. In which case, I figured it would be simplest to pass the Maven-style or Gradle-style string directly, with minimum changes, and parse it in the test runner. The other option would either require defining a custom format, for no good reason, or to creatively use regular expressions, which would be doable, but those regexps would be relatively complex, because they would have to express in one place what we currently can express with multiple relatively simple regexps.
This commit adds support for selecting tests to run during continuous testing that is very close to the native test filtering capability of the given build system. That is, in Maven, there's support for the `-Dtest=...` system property, while in Gradle, there's support for the `--tests` command line option. Format of these options follows the format used by the build system.
285f414
to
528ae6b
Compare
I rebased to check if I finally fixed all the CI issues. Fingers crossed. |
Status for workflow
|
Status for workflow
|
Thanks! I've attempted investigation in my fork, but couldn't find anything, and now it works :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Ladicek , thanks!
This commit adds support for selecting tests to run during continuous testing that is very close to the native test filtering capability of the given build system. That is, in Maven, there's support for the
-Dtest=...
system property, while in Gradle, there's support for the--tests
command line option. Format of these options follows the format used by the build system.Fixes #20231