test: Fix restClientFixture test flaking #4484
Conversation
ghost
added
the
| @@ -328,7 +329,7 @@ func (f *RestClientFixture) SendMoneyAndWait(curRound, amountToSend, transaction | |||
| // SendMoneyAndWaitFromWallet is as above, but for a specific wallet | |||
| func (f *RestClientFixture) SendMoneyAndWaitFromWallet(walletHandle, walletPassword []byte, curRound, amountToSend, transactionFee uint64, fromAccount, toAccount string, closeToAccount string) (txn v1.Transaction) { | |||
| client := f.LibGoalClient | |||
| fundingTx, err := client.SendPaymentFromWallet(walletHandle, walletPassword, fromAccount, toAccount, transactionFee, amountToSend, nil, closeToAccount, 0, 0) | |||
| fundingTx, err := client.SendPaymentFromWallet(walletHandle, walletPassword, fromAccount, toAccount, transactionFee, amountToSend, nil, closeToAccount, basics.Round(curRound).SubSaturate(1), basics.Round(curRound+1000)) | |||
There was a problem hiding this comment.
| fundingTx, err := client.SendPaymentFromWallet(walletHandle, walletPassword, fromAccount, toAccount, transactionFee, amountToSend, nil, closeToAccount, basics.Round(curRound).SubSaturate(1), basics.Round(curRound+1000)) | |
| fundingTx, err := client.SendPaymentFromWallet(walletHandle, walletPassword, fromAccount, toAccount, transactionFee, amountToSend, nil, closeToAccount, basics.Round(curRound).SubSaturate(1), 0) |
I guess it is better to let the API to determine last valid that is proto (MaxTxnLife) dependant
There was a problem hiding this comment.
Sounds good. Another question: do you think its reasonable to just use basics.Round(curRound).SubSaturate(1)?
There was a problem hiding this comment.
basics.Round(curRound).SubSaturate(1) looks good to me as first valid
There was a problem hiding this comment.
looks like the problem we get curRound too early, and then it takes 3+ rounds more to fill rest of fields and submit the txn. Well, please set last valid to firstValid + maxTxnLife like computeValidityRounds does.
There was a problem hiding this comment.
The problem was that for that specific test, it inputted current round as current round + 5.
ghost
marked this pull request as ready for review
Codecov Report
@@ Coverage Diff @@
## master #4484 +/- ##
==========================================
+ Coverage 55.23% 55.25% +0.01%
==========================================
Files 398 398
Lines 50297 50297
==========================================
+ Hits 27784 27791 +7
+ Misses 20180 20177 -3
+ Partials 2333 2329 -4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Summary
Many tests that use the restClientFixture flake for the following reason: The test spins up 2 nodes, Node A and Node B. Node A observes it is currently round r, but node B is still on round r-1. Node A then sends a tx with validity range (r, r+1000). Node B then discards the tx since r-1 is outside of the tx’s validity range. Then the test fails because the tx's are never processed if the proposer of each round happens to be Node B.
This test fixes the issue by changing the validity window.
Test Plan
This is a test.