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

Reduce the number of cluster start/stops for Metadata end to end tests #1186

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

peternied
Copy link
Member

@peternied peternied commented Dec 9, 2024

Description

After recently reviewing metadata migration end to end test cases I
noticed there was 56 different tests, this was a large increase, taking
nearly 20 minutes of runtime. Reviewing the parameters of the tests
I'm running evaluate and then immediately following migrate, this
should reduce the test cases by nearly half and increase of coverage
using http and snapshot sources.

I have tweaked how we use templates moving back to the logic of
legacy templates for ES 6.X and using both index and index component
templates for future ES versions, which should be another 30%
reduction.

Issues

Improvement

Previous

Package Tests Failures Ignored Duration Success rate
org.opensearch.migrations 56 0 0 20m54.98s 100%

This Change

Package Tests Failures Ignored Duration Success rate
org.opensearch.migrations 24 0 0 9m43.71s 100%

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Package Tests Failures Ignored Duration Success rate
org.opensearch.migrations 56 0 0 20m54.98s 100%Package Tests Failures Ignored Duration Success rate
org.opensearch.migrations 24 0 0 9m43.71s 100%

After recently reviewing metadata migration end to end test cases I
noticed there was 56 different tests, this was a large increase, taking
nearly 20 minutes of runtime.  Reviewing the parameters of the tests
I'm running evaluate and then immediately following migrate, this
should reduce the test cases by nearly half and increase of coverage
using http and snapshot sources.

I have tweaked how we use templates moving back to the logic of
legacy templates for ES 6.X and using both index and index compontent
templates for future ES versions, which should be another 30%
redunction.

Signed-off-by: Peter Nied <peternied@hotmail.com>
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.97%. Comparing base (fc62f57) to head (ec2bb14).
Report is 57 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1186      +/-   ##
============================================
+ Coverage     80.93%   80.97%   +0.03%     
- Complexity     2995     2998       +3     
============================================
  Files           407      407              
  Lines         15241    15241              
  Branches       1021     1021              
============================================
+ Hits          12336    12341       +5     
+ Misses         2277     2275       -2     
+ Partials        628      625       -3     
Flag Coverage Δ
unittests 80.97% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@peternied
Copy link
Member Author

@AndreKurait could you take a look at this change since you most recently increased the number of test cases? Note; the code coverage numbers are unchanged with this version, but 10 minutes faster ⚡

@peternied peternied marked this pull request as ready for review December 9, 2024 20:31
void metadataCommand(ContainerVersion sourceVersion, ContainerVersion targetVersion, TransferMedium medium,
MetadataCommands command, TemplateType templateType) {
void metadataCommand(ContainerVersion sourceVersion, ContainerVersion targetVersion, TransferMedium medium) {
var templateType = VersionMatchers.isES_6_X.test(sourceVersion.getVersion())
Copy link
Member

Choose a reason for hiding this comment

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

Where are you testing TemplateType.Legacy on ES 7? There was a bug in that flow that these tests were added thereafter

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you review this matrix for the test cover for template types? I'll update the code to include the correct set of items

ES Version Template Type(s)
ES 6.X Legacy
ES 7.X Legacy & Template & Component Template
OS 1.X Legacy & Template & Component Template
OS 2.X Legacy & Template & Component Template

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's correct.

There's also a difference from a Template with an inline mapping vs Template linked to a Component Template so they should each be tested.

@AndreKurait AndreKurait merged commit 1b5c6a5 into opensearch-project:main Dec 11, 2024
22 checks passed
@peternied peternied deleted the faster-meta-e2e branch December 11, 2024 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants