Skip to content

Strip out seed from reproduce method name #121987

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

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

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Feb 7, 2025

When -Dtests.iters is used the test seed is appended to the method name by randomized runner. That in turn causes the reproduce line to contain the seed as part of the method name, which makes the reproduce line unuseable. This commit strips off any additional part of the method name before printing.

When -Dtests.iters is used the test seed is appended to the method name
by randomized runner. That in turn causes the reproduce line to contain
the seed as part of the method name, which makes the reproduce line
unuseable. This commit strips off any additional part of the method name
before printing.
@rjernst rjernst added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure auto-backport Automatically create backport pull requests when merged v9.0.0 v8.18.1 v8.19.0 v9.1.0 labels Feb 7, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@elasticsearchmachine elasticsearchmachine added the Team:Delivery Meta label for Delivery team label Feb 7, 2025
@@ -61,6 +61,7 @@ public void testReplacePropertiesPlaceholderSystemProperty() {
.replacePropertyPlaceholders()
.build();
assertThat(settings.get("setting1"), equalTo(value));
fail("foo");
Copy link
Contributor

Choose a reason for hiding this comment

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

should this part also be merged?

Copy link
Member Author

Choose a reason for hiding this comment

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

definitely not! removed

@rjernst rjernst enabled auto-merge (squash) February 7, 2025 01:26
if (methodName != null) {
// Method names only have spaces if used with -Dtests.iters where the seed is appended.
// Here we strip out anything from the first space, leaving the real method name intact.
methodName = methodName.split(" ")[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the case. For things like YAML tests the names very often have spaces. This would then cause those repro lines to be corrupted.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, I had not considered parameterized tests. In fact, the seed is put into the same curly-braced section as parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v8.18.2 v8.19.0 v9.0.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants