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

[Cleanup] Remove legacy skip tags from YAML tests #16574

Open
msfroh opened this issue Nov 5, 2024 · 1 comment
Open

[Cleanup] Remove legacy skip tags from YAML tests #16574

msfroh opened this issue Nov 5, 2024 · 1 comment
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers Other

Comments

@msfroh
Copy link
Collaborator

msfroh commented Nov 5, 2024

Is your feature request related to a problem? Please describe

We sometimes need to skip YAML REST tests when running on old versions of OpenSearch, especially for the backward-compatibility (BWC) tests and mixed-cluster tests.

Mostly, this is for stuff that was not supported on old versions, where a mixed cluster would have nodes running a newer version that support the feature being tested, along with nodes from an earlier version that do not. Some of these skip entries are for predecessor versions, though, which we won't test in a mixed-cluster environment.

Describe the solution you'd like

It would be a nice cleanup to go through and remove the - skip: annotations from YAML REST tests where they refer to predecessor versions.

Here is a search that matches skips for 7.x versions in YAML tests (112 files right now): https://github.com/search?q=repo%3Aopensearch-project%2FOpenSearch+version%3A+%22+-+7%22+language%3AYAML&type=code

Here is a search for skips against 6.x versions (49 files, most checking version - 6.99.99 as a way of checking for 7.0+): https://github.com/search?q=repo%3Aopensearch-project%2FOpenSearch+version%3A+%22+-+6%22+language%3AYAML&type=code

I would suggest addressing these in two separate PRs. Removing the 6.x skips can be backported to the 2.x branch. I don't know if we can remove 7.x checks from the 2.x branch -- it looks like the code assumes 2.x is backward compatible with 7.10:

protected Version computeMinCompatVersion() {
if (major == 1 || major == 7) {
// we don't have LegacyESVersion.V_6 constants, so set it to its last minor
return LegacyESVersion.fromId(6080099);
} else if (major == 2) {
return LegacyESVersion.fromId(7100099);

Related component

Other

Describe alternatives you've considered

These skips aren't really hurting anyone. We could just leave them in place.

Eventually, though, if/when we release OpenSearch 5.0 and switch the main branch to 6.0, they might cause us to skip tests that we don't want to skip.

Additional context

No response

@msfroh msfroh added enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers untriaged labels Nov 5, 2024
@github-actions github-actions bot added the Other label Nov 5, 2024
@rgsriram
Copy link

rgsriram commented Nov 13, 2024

I like to contribute. Please assign it to me.

@msfroh msfroh assigned msfroh and rgsriram and unassigned msfroh Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers Other
Projects
None yet
Development

No branches or pull requests

3 participants