-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
base: main
Are you sure you want to change the base?
Conversation
REST tests extending JsonLogsIntegTestCase migrated to the new REST testing framework, using 'elasticsearch.internal-java-rest-test' Gradle plugin
} | ||
// 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? |
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.
Is this special? Will the REST tests use the correct distribution without this configuration?
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); | ||
} | ||
}); |
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 haven't recreated this behaviour. Do we run these tests against external clusters? Do we need this?
.settingsModifier( | ||
settings -> settings.entrySet() | ||
.stream() | ||
.filter(it -> it.getKey().equals("node.name") == false) | ||
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)) | ||
) |
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'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.
REST tests extending JsonLogsIntegTestCase migrated to the new REST testing framework, using 'elasticsearch.internal-java-rest-test' Gradle plugin