-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix tests in IndexLifecycleServiceTests
#129449
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
Conversation
These two tests were ineffective because the result of the countdown latch was ignored. Correctly asserting the output of the latch showed that the test was failing, which was due to missing test setup - the policies were not defined in the registry and the lifecycle runner didn't execute the cluster state steps due to a missing task queue. Finally, some test objects were constructed incorrectly likely due to typos.
|
Pinging @elastic/es-data-management (Team:Data Management) |
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.
Pull Request Overview
This PR fixes test failures in IndexLifecycleServiceTests by correcting test object construction, fixing latch and exception assignment, and setting up cluster services properly.
- Correct test object initialization by using the proper lifecycle phase and policy variables.
- Update latch and exception configuration to target the correct mock step.
- Enhance test setup by initializing cluster settings and a ClusterService to simulate the environment more faithfully.
Comments suppressed due to low confidence (3)
x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleServiceTests.java:441
- [nitpick] The creation of cluster settings and the ClusterService is critical for simulating the environment. Consider refactoring this setup into a common helper if similar configuration is used in other tests to improve maintainability.
ClusterSettings clusterSettings = new ClusterSettings(
settings,
Sets.union(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS, Set.of(LifecycleSettings.LIFECYCLE_POLL_INTERVAL_SETTING))
);
x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleServiceTests.java:382
- The test object construction has been corrected to use the appropriate phase variable. Verify that mapping i2phase to policy2 is intended to reflect the right test scenario.
LifecyclePolicy i2policy = newTestLifecyclePolicy(policy2, Map.of(i2phase.getName(), i2phase));
x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleServiceTests.java:408
- The assignment of the latch (and subsequent exception setting) to i2mockStep appears to be a correction from the previous misuse of i1mockStep. Please confirm that this change accurately simulates the intended cluster state action steps.
((IndexLifecycleRunnerTests.MockClusterStateActionStep) i2mockStep).setLatch(stepLatch);
lukewhiting
left a comment
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.
Looks good 👍🏻
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
These two tests were ineffective because the result of the countdown latch was ignored. Correctly asserting the output of the latch showed that the test was failing, which was due to missing test setup - the policies were not defined in the registry and the lifecycle runner didn't execute the cluster state steps due to a missing task queue. Finally, some test objects were constructed incorrectly likely due to typos. (cherry picked from commit d384e2f)
These two tests were ineffective because the result of the countdown latch was ignored. Correctly asserting the output of the latch showed that the test was failing, which was due to missing test setup - the policies were not defined in the registry and the lifecycle runner didn't execute the cluster state steps due to a missing task queue. Finally, some test objects were constructed incorrectly likely due to typos.