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

[TEST] Migrated REST tests extending JsonLogsIntegTestCase #115188

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

Conversation

jozala
Copy link
Contributor

@jozala jozala commented Oct 21, 2024

REST tests extending JsonLogsIntegTestCase migrated to the new REST testing framework, using 'elasticsearch.internal-java-rest-test' Gradle plugin

REST tests extending JsonLogsIntegTestCase migrated to the new REST
testing framework, using 'elasticsearch.internal-java-rest-test' Gradle
plugin
@jozala jozala added >non-issue >test Issues or PRs that are addressing/adding tests Team:Delivery Meta label for Delivery team v8.17.0 :Delivery/Build Build or test infrastructure labels Oct 21, 2024
}
// TODO @jozala - not sure what to do about this --external--. Do we ever run against external clusters?
//tasks.named("javaRestTest").configure {
// dependsOn "assemble" // TODO @jozala - is this needed so that it uses the right distribution? Am I testing here with the expected distribution?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this special? Will the REST tests use the correct distribution without this configuration?

Comment on lines -33 to -43
assumeFalse(
"Skipping test because it is being run against an external cluster.",
logFile.getFileName().toString().equals("--external--")
);
return AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
try {
return Files.newBufferedReader(logFile, StandardCharsets.UTF_8);
} catch (IOException e) {
throw new RuntimeException(e);
}
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't recreated this behaviour. Do we run these tests against external clusters? Do we need this?

Comment on lines +39 to +44
.settingsModifier(
settings -> settings.entrySet()
.stream()
.filter(it -> it.getKey().equals("node.name") == false)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))
)
Copy link
Contributor Author

@jozala jozala Oct 21, 2024

Choose a reason for hiding this comment

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

I've added this settings modifier in ClusterSpecBuilder to be able to remove node.name from configuration, but keep the rest of the Elasticsearch node setup working. I am wondering if there is already a better way to achieve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants