-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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
Enable longrunning tests to run #19208
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this resulted in a week of delay for Vlad/Louis
? @zelig
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
longrunning
flag, 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.