-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix flaky test in MetadataIndexTemplateServiceTests #20267
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Joe Liu <[email protected]>
WalkthroughThe changes refactor two test assertions in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| ex.getMessage().contains("specifies a context which cannot be used without enabling") | ||
| "Invalid exception message." + msg, | ||
| msg.contains("specifies a context which cannot be used without enabling") | ||
| || msg.contains("specifies a context which is not loaded on the cluster") |
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.
Wouldn't this be the wrong assertion for testPutGlobalV2TemplateWhichProvidesContextWithContextDisabled? I think it would be better to understand why this is being thrown to prevent it.
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.
Maybe the failing test needs to be annotated with @LockFeatureFlag(APPLICATION_BASED_CONFIGURATION_TEMPLATES)?
|
❌ Gradle check result for cacc32a: null 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? |
Description
Fix a flaky test where there is a test expectation mismatch.
the test asserts that the thrown InvalidIndexTemplateException message contains:
"specifies a context which cannot be used without enabling"But the current code path throws a different message:
"specifies a context which is not loaded on the cluster."It typically happens when the validation order changed or a different validation branch triggers first instead of “context feature disabled”, it now fails earlier/later with “context not loaded”.
Related Issues
Resolves #19058
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.