-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix LocalTranslogTests.testWithRandomException test scenario #6748
Conversation
Gradle Check (Jenkins) Run Completed with:
|
Please sign DCO/iterate to green. Thanks! |
Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>
Done) |
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #6748 +/- ##
============================================
- Coverage 70.77% 70.76% -0.01%
+ Complexity 59270 59236 -34
============================================
Files 4822 4822
Lines 283940 283940
Branches 40947 40947
============================================
- Hits 200963 200935 -28
+ Misses 66454 66402 -52
- Partials 16523 16603 +80 see 473 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@mitrofmep Do you think you could add a test that always hits this code? |
Sure! Proceeding from this case, is it ok if I'll create a copy of this test |
Gradle Check (Jenkins) Run Completed with:
|
Hey @dblock I independently tracked this one down, and the existing random tests do trigger this; it's 100% reproducible with the seed given in the failed log builds. (Call heirarchy confirms it fails 1 in 100 times.) I don't think an additional test is needed, and I think the fix is correct here (the test expects the failure tracked in "tragic" but it's just a plain exception. |
I might note that the "expected" and "actual" fields in the |
Let's agree to disagree. When tests pass 99% of the time means we can introduce a regression and catch it only 1% of the time (we got lucky here!). I'll merge either way. |
@mitrofmep LMK if you want me to merge as is or fix the order of the check that @dbwiddis suggests (and write a test that always fails without this fix as I do? :)) |
39758ea
to
42f2d8a
Compare
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Hello, @dblock! Could you merge it as is for now, and we can add this always falling test later. I also reversed "actual" and "expected" fields, as @dbwiddis mentioned. |
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
@dblock, is it okay for merging now, or should I add something else? |
* fix #6625 Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> * fix #6625 - reverse assertEquals params Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> --------- Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> (cherry picked from commit 4afc914) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…7263) * fix #6625 * fix #6625 - reverse assertEquals params --------- (cherry picked from commit 4afc914) Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…rch-project#6748) * fix opensearch-project#6625 Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> * fix opensearch-project#6625 - reverse assertEquals params Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> --------- Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>
…rch-project#6748) * fix opensearch-project#6625 Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> * fix opensearch-project#6625 - reverse assertEquals params Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> --------- Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Description
Set TragicException in case of simulated Runtime Exception for asserting and passing test
LocalTranslogTests.testWithRandomException
Issues Resolved
Resolves #6625
Check List