test: Resume wait correctly following test node restart #1515
Conversation
This isn't really required for the fix, but I made the change whilst investigating and think it is a good one (avoids a sleep on non-p2p tests, and matches/scales the P2P sleeps as they do on normal/non-restart config)
Previously the full wait-cycle would be calculated, meaning that if a Wait action was before the Restart action the Second wait action would progress once the expected number of events for the first Wait was recieved. This changes it so that upon restart the wait calculation is started from just after the last wait action before the restart.
AndrewSisley
added
area/testing
Codecov Report
@@ Coverage Diff @@
## develop #1515 +/- ##
===========================================
- Coverage 72.20% 72.03% -0.17%
===========================================
Files 185 185
Lines 18295 18295
===========================================
- Hits 13210 13179 -31
- Misses 4046 4069 +23
- Partials 1039 1047 +8 |
| actionLoop: | ||
| for i := actionIndex; i >= 0; i-- { | ||
| switch testCase.Actions[i].(type) { | ||
| case WaitForSync: |
There was a problem hiding this comment.
question: for all other cases (default case for example) doesn't matter?
There was a problem hiding this comment.
For all other cases, the value of waitGroupStartIndex would remain 0 - the start of the slice
| syncChans := []chan struct{}{} | ||
| for _, tc := range testCase.Actions { | ||
| switch action := tc.(type) { | ||
| case ConnectPeers: | ||
| // Give the nodes a chance to connect to each other and learn about each other's subscribed topics. | ||
| time.Sleep(100 * time.Millisecond) |
There was a problem hiding this comment.
nitpick: Perhaps store this as a constant somewhere at the top of this file?
There was a problem hiding this comment.
Really not a fan of that - it just means anyone reading has to break flow and scroll to the top of a file in order to view the value of this arbitrary value.
| )) | ||
| case ConfigureReplicator: | ||
| // Give the nodes a chance to connect to each other and learn about each other's subscribed topics. | ||
| time.Sleep(100 * time.Millisecond) |
There was a problem hiding this comment.
nitpick: Perhaps store this as a constant somewhere at the top of this file?
There was a problem hiding this comment.
Really not a fan of that - it just means anyone reading has to break flow and scroll to the top of a file in order to view the value of this arbitrary value.
Thanks, and thanks for your time and help on this too :) |
…k#1515) ## Relevant issue(s) Resolves sourcenetwork#1506 ## Description Previously the full wait-cycle would be calculated, meaning that if a Wait action was before the Restart action the Second wait action would progress once the expected number of events for the first Wait was received. This changes it so that upon restart the wait calculation is started from just after the last wait action before the restart.
Relevant issue(s)
Resolves #1506
Description
Previously the full wait-cycle would be calculated, meaning that if a Wait action was before the Restart action the Second wait action would progress once the expected number of events for the first Wait was received. This changes it so that upon restart the wait calculation is started from just after the last wait action before the restart.
How has this been tested?
Had a few hundred successful runs of the only affect test i(
TestP2PPeerReplicatorWithUpdateAndRestart) n the CI.