Skip to content

Fix index pruning YAML test flakiness - #5813

Open
dai-chen wants to merge 1 commit into
opensearch-project:mainfrom
dai-chen:fix/index-pruning-yaml-flakes
Open

dai-chen wants to merge 1 commit into
opensearch-project:mainfrom
dai-chen:fix/index-pruning-yaml-flakes

Conversation

@dai-chen

@dai-chen dai-chen commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Stabilizes index-pruning YAML tests:

  • Close active PITs before reading point_in_time_total, avoiding the cleanup race seen in this Windows CI failure.
  • Allow the exact legacy global-template overlap warnings, addressing the randomized setup seen in this Linux CI failure.

Related Issues

N/A

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

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.

@dai-chen dai-chen self-assigned this Sep 25, 2026
@dai-chen dai-chen added the testing Related to improving software testing label Sep 25, 2026
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 1d954c1)

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to 1d954c1
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Handle missing PITs gracefully

Add error handling for the delete_all_pits operation to prevent test failures if no
PITs exist. Consider using catch or ignore directives to handle cases where there
are no active PITs to delete, ensuring test stability.

integ-test/src/yamlRestTest/resources/rest-api-spec/test/ppl/index_pruning.yml [88-89]

 - do:
+    catch: missing
     delete_all_pits: {}
Suggestion importance[1-10]: 3

__

Why: While adding error handling for delete_all_pits could improve robustness, the suggestion assumes PITs might not exist when they should based on the test flow. The operation follows a query that creates a PIT, so the error case is unlikely. The suggestion provides marginal defensive programming value but isn't critical for test correctness.

Low

Previous suggestions

Suggestions up to commit 52a5f86
CategorySuggestion                                                                                                                                    Impact
General
Verify PIT deletion success

Add error handling or verification after delete_all_pits to ensure PITs are
successfully deleted before reading stats. Without confirmation, the test could
still read stale metrics if the deletion fails silently, leading to flaky test
results.

integ-test/src/yamlRestTest/resources/rest-api-spec/test/ppl/index_pruning.yml [87-88]

 - do:
     delete_all_pits: {}
+- match: {acknowledged: true}
 - do:
     indices.stats: {index: 'pruning-it-*', metric: search, filter_path: 'indices.*.total.search.point_in_time_total'}
Suggestion importance[1-10]: 5

__

Why: While adding verification after delete_all_pits could improve test reliability, the suggestion assumes the operation returns an acknowledged field without confirming this is the actual API response structure. The improvement is valid in principle but may need adjustment based on the actual API contract.

Low

@dai-chen dai-chen added infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. and removed testing Related to improving software testing labels Sep 25, 2026
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.23%. Comparing base (07f079d) to head (1d954c1).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5813      +/-   ##
============================================
- Coverage     63.26%   63.23%   -0.03%     
  Complexity     8823     8823              
============================================
  Files           938      938              
  Lines         40217    40236      +19     
  Branches       4532     4537       +5     
============================================
+ Hits          25442    25445       +3     
- Misses        13951    13966      +15     
- Partials        824      825       +1     
Flag Coverage Δ
sql-engine 63.23% <ø> (-0.03%) ⬇️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Chen Dai <daichen@amazon.com>
@dai-chen
dai-chen force-pushed the fix/index-pruning-yaml-flakes branch from 52a5f86 to 1d954c1 Compare September 25, 2026 20:36
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 1d954c1

@dai-chen

Copy link
Copy Markdown
Collaborator Author

First run all passed. Modified the comments in yaml file slightly to trigger the second run.

@dai-chen

Copy link
Copy Markdown
Collaborator Author

First run all passed. Modified the comments in yaml file slightly to trigger the second run.

Second run passed too.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants