-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Convert packaging upgrade tests to java #60560
Conversation
This commit removes the last of the bats tests, converting the rpm/deb upgrade tests to java. It adds a new pattern of tasks, similar in nature but separate from the existing distro tests, named `distroUpgradeTest`. For each index compatible version, a `distroUpgradeTest.VERSION` task exxists. Each distribution then has a task, named `distroUpgradeTest.VERSION.DISTRO`. One thing to note is these new tests do not cover no-jdk versions of the rpm/deb packages, since the distribution/bwc project does not currently build those. closes elastic#59145 closes elastic#46005
Pinging @elastic/es-core-infra (:Core/Infra/Packaging) |
Another thing to note is this change only provides the new distroUpgradeTest tasks. We still need CI jobs then added. Currently the bats upgrade tests operate by choosing a random version to upgrade from, but these tests have been causing packaging test failures since they were recently re-enabled (see #59145). I propose to merge this change before we have CI setup, since the tests are currently completely broken anyways. We can then assess whether it is worth the time to try another matrix job in jenkins, or wait for TeamCity for the new job. |
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.
LGTM, happy that this is a big net reduction in code.
@@ -1,131 +0,0 @@ | |||
/* |
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.
Yay!
Map<String, TaskProvider<?>> depsTasks = new HashMap<>(); | ||
for (ElasticsearchDistribution distribution : testDistributions) { | ||
String taskname = destructiveDistroTestTaskName(distribution); | ||
TaskProvider<?> depsTask = project.getTasks().register(taskname + "#deps"); |
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'm now wondering if this intermediary task is striclty necessary. Is there anyreason why the test task can't just depend directly on the distribution, and the VM tasks depend on all distributions?
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 don't think we want the VM task depending on all distributions. That would mean a repro line would rebuild all distros, instead of just the one you are trying to run.
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.
Ok, I got confused by the fact we are passing all the deps tasks to the wrapper task creation. Couldn't we do the same thing though, and directly bucket the distributions, instead of the deps tasks? Just seems another layer we don't really need.
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.
We would need to apply the distro download plugin inside the subprojects (ie the ones with the wrapper tasks), as well as produce the test jar as an artifact. However, that is a lot of extra work that is not useful since we don't actually need those in the subproject to run, we only want to ensure the tasks in :qa:os
ran before launching the VM task.
@@ -323,69 +295,52 @@ private static Configuration configureExamplePlugin(Project project) { | |||
return examplePlugin; | |||
} | |||
|
|||
private static TaskProvider<GradleDistroTestTask> configureVMWrapperTask( | |||
private static void configureWrapperTasks( |
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.
Why rename this? I think having "VM" in the name is helpful. Else let's add a comment.
This commit removes the last of the bats tests, converting the rpm/deb upgrade tests to java. It adds a new pattern of tasks, similar in nature but separate from the existing distro tests, named `distroUpgradeTest`. For each index compatible version, a `distroUpgradeTest.VERSION` task exxists. Each distribution then has a task, named `distroUpgradeTest.VERSION.DISTRO`. One thing to note is these new tests do not cover no-jdk versions of the rpm/deb packages, since the distribution/bwc project does not currently build those. closes elastic#59145 closes elastic#46005
This commit removes the last of the bats tests, converting the rpm/deb upgrade tests to java. It adds a new pattern of tasks, similar in nature but separate from the existing distro tests, named `distroUpgradeTest`. For each index compatible version, a `distroUpgradeTest.VERSION` task exxists. Each distribution then has a task, named `distroUpgradeTest.VERSION.DISTRO`. One thing to note is these new tests do not cover no-jdk versions of the rpm/deb packages, since the distribution/bwc project does not currently build those. closes #59145 closes #46005
This commit removes the last of the bats tests, converting the rpm/deb upgrade tests to java. It adds a new pattern of tasks, similar in nature but separate from the existing distro tests, named `distroUpgradeTest`. For each index compatible version, a `distroUpgradeTest.VERSION` task exxists. Each distribution then has a task, named `distroUpgradeTest.VERSION.DISTRO`. One thing to note is these new tests do not cover no-jdk versions of the rpm/deb packages, since the distribution/bwc project does not currently build those. closes #59145 closes #46005
This commit removes the last of the bats tests, converting the rpm/deb
upgrade tests to java. It adds a new pattern of tasks, similar in nature
but separate from the existing distro tests, named
distroUpgradeTest
.For each index compatible version, a
distroUpgradeTest.VERSION
taskexxists. Each distribution then has a task, named
distroUpgradeTest.VERSION.DISTRO
.One thing to note is these new tests do not cover no-jdk versions of
the rpm/deb packages, since the distribution/bwc project does not
currently build those.
closes #59145
closes #46005