Skip to content

Migrate x-pack-deprecation REST tests #131444

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 11 commits into
base: main
Choose a base branch
from

Conversation

ldematte
Copy link
Contributor

@ldematte ldematte commented Jul 17, 2025

This PR migrates legacy rest tests in the x-pack deprecation module.

I tried to use the INTEG_TEST distribution and add just the basic modules needed (ml, data-streams, deprecation) but this way the indices are not persisted and the tests fails. Need more investigation to identify the set of required modules.

Also, I preferred not to move and "flatten" the project here, but maintain a separate "qa" sub-project, as the IT tests use two different test plugins, specific to this module.

@ldematte ldematte requested a review from a team July 17, 2025 13:18
@ldematte ldematte added :Core/Infra/Core Core issues without another label >refactoring auto-backport Automatically create backport pull requests when merged v9.1.1 v8.19.1 labels Jul 17, 2025
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.2.0 labels Jul 17, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@ldematte ldematte requested a review from a team as a code owner July 17, 2025 13:23
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 but one comment:

Also, I preferred not to move and "flatten" the project here, but maintain a separate "qa" sub-project, as the IT tests use two different test plugins, specific to this module.

Could we at least combine to two separate QA projects into one? I don't see any reason to keep them separate.

@rjernst
Copy link
Member

rjernst commented Jul 17, 2025

as the IT tests use two different test plugins

FWIW this is a big advantage of the new IT tests, that we can control plugins for the cluster per test suite. Moving the qa tests into one sourceset doesn't affect that, but does make the entire set of tests more discoverable together.

@ldematte
Copy link
Contributor Author

ldematte commented Jul 18, 2025

Could we at least combine to two separate QA projects into one? I don't see any reason to keep them separate.

Oh yes, definitely, I can do that.

FWIW this is a big advantage of the new IT tests, that we can control plugins for the cluster per test suite. Moving the qa tests into one sourceset doesn't affect that, but does make the entire set of tests more discoverable together.

What I cannot (or don't want) to do is to remove the QA sub-project altogether: keep me honest here and correct me if I'm wrong, but if I remove it and move everything under :x-pack:deprecation I have to find a new "home" for the plugins source. And I'm reluctant to do that in this case, because the plugins are very specific to these tests.

@ldematte
Copy link
Contributor Author

Could we at least combine to two separate QA projects into one? I don't see any reason to keep them separate.

Merged them. @mark-vieira still LG to you?

@mark-vieira
Copy link
Contributor

I'm pretty sure the module you need here is x-pack-stack, since it's what registers the logs-* index templates.

@ldematte
Copy link
Contributor Author

I'm pretty sure the module you need here is x-pack-stack, since it's what registers the logs-* index templates.

Tried to add it, still failing :/

@mark-vieira
Copy link
Contributor

I'm pretty sure the module you need here is x-pack-stack, since it's what registers the logs-* index templates.

Tried to add it, still failing :/

Same error or a different one? It's often a bit of a whack-a-mole with these things.

@ldematte
Copy link
Contributor Author

Exact same indexnotfoud exception: https://gradle-enterprise.elastic.co/s/ef6jzgb7p337w

@ClassRule
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.plugin("deprecation-plugin")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be installing this as a module not a plugin. Also, it should already be included in the default distro, so installing it as a plugin here is unnecessary. In fact, I'm surprised this even works and doesn't result in an error since we basically have this thing loaded twice.

@mark-vieira
Copy link
Contributor

Exact same indexnotfoud exception: https://gradle-enterprise.elastic.co/s/ef6jzgb7p337w

Hmm. Yeah, there's something registering that index. I would expect it to be the deprecation plugin itself. Perhaps the test is implicitly relying on something to trigger a deprecation and therefore create the index? I'm not sure.

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 :Core/Infra/Core Core issues without another label >refactoring Team:Core/Infra Meta label for core/infra team v8.19.1 v9.1.1 v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants