Enable longrunning tests to run #19208
Conversation
holisticode
requested review from
janos,
acud,
nonsense and
zelig
as code owners
| if realCount/2 != emc { | ||
| return fmt.Errorf("Real subscriptions and expected amount don't match; real: %d, expected: %d", realCount/2, emc) | ||
| if realCount != emc { | ||
| return fmt.Errorf("Real subscriptions and expected amount don't match; real: %d, expected: %d", realCount, emc) |
There was a problem hiding this comment.
please use this standard way of formulating test fails:
"incorrect number of subscriptions. expected %v, got %v."
| @@ -134,6 +134,9 @@ func netStoreAndDeliveryWithAddr(ctx *adapters.ServiceContext, bucket *sync.Map, | |||
| bucket.Store(bucketKeyDB, netStore) | |||
| bucket.Store(bucketKeyDelivery, delivery) | |||
| bucket.Store(bucketKeyFileStore, fileStore) | |||
| // for the kademlia object, we use the global key from the simulation package, | |||
| // as the simulation will try to access it in the WaitTillHealthy with that key | |||
| bucket.Store(simulation.BucketKeyKademlia, kad) | |||
There was a problem hiding this comment.
I did not realise this requirement of using simulation/waitTillHealthy. Since this resulted in a week of delay for Vlad/Louis, I think we should handle this some other way or document it better.
There was a problem hiding this comment.
Since this resulted in a week of delay for Vlad/Louis
? @zelig
There was a problem hiding this comment.
Wow so long delay, I thought that I added some comments regarding that, but this comment is very badly written https://github.com/ethereum/go-ethereum/blob/master/swarm/network/simulation/kademlia.go#L32. I am sorry for causing problems of this kind. Maybe we should have a better approach for this functionality, it is not clear at all, causing a lot of pain. Sorry.
There was a problem hiding this comment.
This example https://godoc.org/github.com/ethereum/go-ethereum/swarm/network/simulation#example-Simulation-WaitTillHealthy may be of some help, but still BucketKeyKademlia/WaitTillHealthy usage over time shows that it is a very bad design.
|
Please prepend package names before merging PRs. |
* p2p/simulations: increased snapshot load timeout for debugging * swarm/network/stream: less nodes for snapshot longrunning tests * swarm/network: fixed longrunning tests * swarm/network/stream: store kademlia in bucket * swarm/network/stream: disabled healthy check in delivery tests * swarm/network/stream: longer SyncUpdateDelay for longrunning tests * swarm/network/stream: more debug output * swarm/network/stream: reduced longrunning snapshot tests to 64 nodes * swarm/network/stream: don't WaitTillHealthy in SyncerSimulation * swarm/network/stream: cleanup for PR
* p2p/simulations: increased snapshot load timeout for debugging * swarm/network/stream: less nodes for snapshot longrunning tests * swarm/network: fixed longrunning tests * swarm/network/stream: store kademlia in bucket * swarm/network/stream: disabled healthy check in delivery tests * swarm/network/stream: longer SyncUpdateDelay for longrunning tests * swarm/network/stream: more debug output * swarm/network/stream: reduced longrunning snapshot tests to 64 nodes * swarm/network/stream: don't WaitTillHealthy in SyncerSimulation * swarm/network/stream: cleanup for PR
This PR introduces a series of bug fixes which would prevent longrunning tests to succeed.
These tests need to be enabled with the
longrunningflag, and were up to today ignored.We can deploy these tests now to the kubernetes test environment, allowing to have better insights about network behavior with bigger networks.