Skip to content
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

fix: flaky test by skipping connectivity checks #955

Closed
wants to merge 1 commit into from

Conversation

dennis-tra
Copy link
Contributor

By skipping connectivity checks we reduce the chances of simultaneously opening a stream that will block connection establishment.

Context: libp2p/go-libp2p#2589

RequestConcurrency: 3, // MAGIC
RequestTimeout: time.Minute, // MAGIC
DefaultQuorum: 0, // MAGIC
SkipConnectivityCheck: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this rather be on a RoutingConfig struct?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

What about just using the following to make the RoutingConfig usable externally

type RoutingConfig = internal.coord.RoutingConfig

By skipping connectivity checks we reduce the chances of simultaneously opening a stream that will block connection establishment.

Context: libp2p/go-libp2p#2589
@dennis-tra dennis-tra force-pushed the v2-flaky-TestQuorumReachedPrematurely branch from 559a162 to 042bba3 Compare September 29, 2023 16:00
@iand
Copy link

iand commented Sep 29, 2023

Maybe a better solution for your test is to allow a Coordinator to be created with a NoopRoutingBehaviour. The behaviours are intended to be modular and replaceable but we haven't exposed a convenient way of doing that.

@iand iand added the v2 All issues related to the v2 rewrite label Oct 2, 2023
@iand
Copy link

iand commented Oct 2, 2023

As per #954 this needs to be applied to zikade

@guillaumemichel
Copy link
Contributor

Closing as v2 has moved to Zikade

See probe-lab/zikade#77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 All issues related to the v2 rewrite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants