-
Notifications
You must be signed in to change notification settings - Fork 24.6k
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
Reapply "Making yaml tests version selector parser compatible with versions returned by Build" #100953
Reapply "Making yaml tests version selector parser compatible with versions returned by Build" #100953
Conversation
…parsing-yaml-tests
…parsing-yaml-tests
…patible-version-parsing-yaml-tests
Pinging @elastic/es-core-infra (Team:Core/Infra) |
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 think we can simplify a little bit here, but I may be nitpicking and overall this looks good to me.
test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java
Outdated
Show resolved
Hide resolved
Version version = Version.fromString(node.getVersion()); | ||
boolean skip = skipVersionRanges.stream().anyMatch(v -> v.contains(version)); | ||
if (false == skip) { | ||
if (nodeMatcher.test(versionString) == false) { |
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.
If we do replace("-SNAPSHOT", "")
here, the possible matchers both get simplified.
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.
Yes, it could become a simple string equals. Same as above, I will change it - it should work, and if not CI test failures will tell us.
@elasticmachine rerun elasticsearch-ci/check-serverless-submodule |
…version-parsing-yaml-tests
Thank you William! |
Reapply #100794
This will need to be merged (again) after #100947 and then be followed by #100746 OR follow quickly #100746 to avoid disruption in serverless.