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

Convert packaging upgrade tests to java #60560

Merged
merged 3 commits into from
Aug 3, 2020
Merged

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Aug 1, 2020

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 elastic#59145
closes elastic#46005
@rjernst rjernst added >test Issues or PRs that are addressing/adding tests :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v8.0.0 v7.10.0 labels Aug 1, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Packaging)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Aug 1, 2020
@rjernst rjernst requested a review from mark-vieira August 1, 2020 02:06
@rjernst
Copy link
Member Author

rjernst commented Aug 1, 2020

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.

Copy link
Contributor

@mark-vieira mark-vieira left a 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 @@
/*
Copy link
Contributor

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");
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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.

Copy link
Member Author

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(
Copy link
Contributor

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.

@rjernst rjernst merged commit dfe42e3 into elastic:master Aug 3, 2020
@rjernst rjernst deleted the distro_tests55 branch August 3, 2020 23:31
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Aug 4, 2020
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
rjernst added a commit that referenced this pull request Aug 5, 2020
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
rjernst added a commit that referenced this pull request Aug 6, 2020
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
@rjernst rjernst added the v7.9.0 label Aug 6, 2020
@mark-vieira mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v7.9.0 v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] :qa:os:destructiveBatsTest.upgrade failures Migrate bats tests to java
4 participants