Ensure all modules are included in INTEG_TEST testcluster distribution#20241
Ensure all modules are included in INTEG_TEST testcluster distribution#20241cwperks merged 3 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: Craig Perkins <cwperx@amazon.com>
|
I'll add a CHANGELOG entry once the CHANGELOG is cleared for 3.5.0. |
WalkthroughReplaces transport-specific distribution outputs and tasks with integ-test equivalents and updates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
❌ Gradle check result for c55e048: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
@cwperks This has already happened. |
|
pushing a new commit, 1 min |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks Are plugins commonly taking dependencies on other modules like autotagging-commons? Or does the server not properly start up without things injected via modules? |
security extends autotagging-commons so this impacts any plugin that has a test with security that uses testclusters instead of docker |
|
❌ Gradle check result for b02fc6b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
@dbwiddis The purpose of this is so that we can replace all instances of |
|
Removing backport tag as #20243 accomplishes that. |
|
❌ Gradle check result for b02fc6b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b02fc6b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b02fc6b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b02fc6b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Gradle check keeps failing on org.opensearch.painless.LangPainlessClientYamlTestSuiteIT.test {yaml=painless/71_context_api/Action to list contexts}. Does not appear to previously have been flaky. Last element of the context array now includes Stash dump on test failure [{
"stash" : {
"body" : {
"contexts" : [
"aggregation_selector",
<snip>
"template",
"terms_set",
"update"
]
}
}
}] |
|
yea the test is written in a way to run against a testcluster and would fail against the min distribution. Updating the test now. |
cwperks
requested review from
Bukhtawar,
CEHENKLE,
anasalkouz and
ashking94
as code owners
cwperks
requested review from
Rishikesh1159,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
owaiskazi19,
reta,
sachinpkale,
saratvemulapalli,
shwetathareja and
sohami
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/71_context_api.yml (1)
4-5: Confirm new index and consider brittleness of order‑based assertionChanging the assertion from a lower index to
contexts.26for"update"makes sense if a new context was inserted earlier, but this test is tightly coupled to the exact ordering of the contexts array. That will break again whenever another context is added or re-ordered.At minimum, please:
- Re‑run
LangPainlessClientYamlTestSuiteIT(or the relevant Gradle task) to confirm thatcontexts.26is the correct index for"update"with the new distribution layout.- Add a short comment explaining that the index is expected to move when contexts are added, to make future updates more obvious.
If the YAML test framework ever supports more flexible matching (e.g., searching by value rather than fixed index), it would be worth refactoring this test at that time to avoid hard‑coding the array position.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/71_context_api.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: check-files
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: detect-breaking-change
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: Analyze (java)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20241 +/- ##
============================================
+ Coverage 73.20% 73.28% +0.08%
- Complexity 71766 71802 +36
============================================
Files 5795 5795
Lines 328302 328303 +1
Branches 47283 47283
============================================
+ Hits 240345 240611 +266
+ Misses 68628 68400 -228
+ Partials 19329 19292 -37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
opensearch-project#20241) * Ensure all modules are included in INTEG_TEST testcluster distribution Signed-off-by: Craig Perkins <cwperx@amazon.com> * Add CHANGELOG entry Signed-off-by: Craig Perkins <cwperx@amazon.com> * Fix test Signed-off-by: Craig Perkins <cwperx@amazon.com> --------- Signed-off-by: Craig Perkins <cwperx@amazon.com>
3 participants
Description
This PR ensures that all modules are included when using the INTEG_TEST testcluster distribution. Currently, when using a testcluster with
INTEG_TESTdistribution, only modules matchingtransport-*are included and as a result many plugins cannot use this distribution adequately because its missing modules likeautotagging-commons.Related Issues
See discussion on #20229
Check List
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.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.