fix flaky test PemTrustConfigTests#testTrustConfigReloadsFileContents #17984
Conversation
Signed-off-by: guojialiang <[email protected]>
guojialiang92
requested review from
anasalkouz,
andrross,
ashking94,
bugmakerrrrrr,
Bukhtawar,
CEHENKLE,
cwperks,
dbwiddis,
gbbafna,
jainankitk,
kotwanikunal,
linuxpi,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
VachaShah
as code owners
|
❕ Gradle check result for 0ee096c: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17984 +/- ##
==========================================
Coverage 72.51% 72.51%
+ Complexity 67108 67101 -7
==========================================
Files 5475 5478 +3
Lines 309916 310034 +118
Branches 45060 45066 +6
==========================================
+ Hits 224725 224823 +98
+ Misses 66895 66844 -51
- Partials 18296 18367 +71 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This failed test and another one testBadFileFormatFails always fail with the same AssertionError in my local, could you help to check if it's caused by the dependency upgrade so the error message thrown is changed? If so I think we can change the assertion in assertFailedToParse() directly.
|
Thanks, @gaobinlong. I abstracted method |
|
❌ Gradle check result for 981586d: 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? |
Signed-off-by: guojialiang <[email protected]>
guojialiang92
force-pushed
the
dev/fix-PemTrustConfigTests-testTrustConfigReloadsFileContents
branch
from
981586d to
37f824e
Compare
|
❌ Gradle check result for 37f824e: 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? |
…gTests-testTrustConfigReloadsFileContents
|
❌ Gradle check result for 67d72db: 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? |
…gTests-testTrustConfigReloadsFileContents
|
❌ Gradle check result for d910aaf: 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? |
Signed-off-by: guojialiang <[email protected]>
| final SslConfigException exception = expectThrows(SslConfigException.class, trustConfig::createTrustManager); | ||
| assertThat(exception.getMessage(), Matchers.containsString(file.toAbsolutePath().toString())); | ||
| assertThat(exception.getMessage(), Matchers.containsString("cannot create trust")); | ||
| assertThat(exception.getMessage(), Matchers.containsString("PEM")); |
There was a problem hiding this comment.
Could we merge the two lines 167 and 168 into one?
There was a problem hiding this comment.
Could combine 166-168 actually.
| boolean isFailedToParse = false; | ||
| boolean isInvalidFileFormat = false; | ||
| try { | ||
| assertFailedToParse(trustConfig, file); |
There was a problem hiding this comment.
Do you know when will Failed to parse happen and when will Invalid format happen? I think it's better to make the trigger condition clear rather than depending on try...catch.
|
@guojialiang92 thanks for the PR! Are you still working on this? |
|
This PR is stalled because it has been open for 30 days with no activity. |
opensearch-trigger-bot
bot
added
stalled
Description
This PR is trying to fix a flakey test
org.opensearch.common.ssl.PemTrustConfigTests#testTrustConfigReloadsFileContents.Related Issues
Resolves #[17983]
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.