-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Skip LLO integration test that flakes #15311
Conversation
What is the goal of this PR? I.e., how removing the "interdependency" would help? |
3194dd7
to
666343e
Compare
@pavel-raykov not sure yet, I am going to try a few things to narrow down the flake |
Flaky Test Detector for
|
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
666343e
to
f44ab8f
Compare
8a46b87
to
842df78
Compare
I see you updated files related to
|
@@ -342,11 +342,6 @@ func TestIntegration_LLO(t *testing.T) { | |||
multiplier := decimal.New(1, 18) | |||
expirationWindow := time.Hour / time.Second | |||
|
|||
reqs := make(chan request, 100000) | |||
serverKey := csakey.MustNewV2XXXTestingOnly(big.NewInt(-1)) | |||
serverPubKey := serverKey.PublicKey |
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.
the t.Run(...) tests do not call t.Parallel() , so I am not sure how moving this to individual tests would help.
Would it make sense to keep this PR only about disabling tests and not modifying them?
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.
No, because if a previous test fails it causes a subsequent test to fail for no good reason if you re-use the same server
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.
Thanks, this makes total sense. In this case, would it be possible to put this stanza inside of startMercuryServer instead of duplicating it?
Requires
Supports