-
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
Eventually should connect because TxManager connection is async #1136
Conversation
core/store/tx_manager_test.go
Outdated
@@ -40,7 +41,11 @@ func TestTxManager_CreateTx_Success(t *testing.T) { | |||
}) | |||
assert.NoError(t, app.StartAndConnect()) | |||
|
|||
gomega.NewGomegaWithT(t).Eventually(func() bool { | |||
return manager.Connected() | |||
}).Should(gomega.Equal(true)) | |||
require.True(t, manager.Connected()) |
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.
Can't you remove this line if you eventually assert it's true in the above line?
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.
Interesting to find this to be an issue, I'm a bit surprised but it makes sense. Should we drop a comment suggesting a future more global fix, since this pattern exists in other tests.
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.
Yeah I was keeping that in to ensure the test aborts, did some testing and gomega does hard fail the test. So removing now.
After some discussion offline, we're waiting on further thought and discussion to see if this indeed fixes the elusive mandelbug issue. |
Capturing for posterity's sake: https://awwapp.com/b/u5gledfmd/# |
CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <[email protected]>
CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <[email protected]>
## Motivation CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues ## Solution add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <[email protected]>
CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <[email protected]>
CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <[email protected]>
CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <[email protected]>
No description provided.