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

webtransport: fix flaky accept queue test #1938

Merged
merged 1 commit into from
Dec 10, 2022

Conversation

marten-seemann
Copy link
Contributor

@marten-seemann marten-seemann commented Dec 8, 2022

Fixes #1870.

The problem here is that it's not valid assumption that the 16 connection we dial first are all accepted (by the QUIC listener) in that order.
What we should assert that out of 17 connections dialed, 16 are accepted and 1 is rejected.

@marten-seemann marten-seemann force-pushed the fix-webtransport-accept-queue-test branch from fe5a7c6 to 32abb4f Compare December 8, 2022 02:35
@marten-seemann marten-seemann marked this pull request as draft December 8, 2022 03:24
@marten-seemann marten-seemann force-pushed the fix-webtransport-accept-queue-test branch from 6f91520 to 7475114 Compare December 8, 2022 05:38
@marten-seemann marten-seemann changed the base branch from master to update-webtransport December 8, 2022 05:38
@marten-seemann marten-seemann marked this pull request as ready for review December 8, 2022 05:39
@marten-seemann marten-seemann force-pushed the fix-webtransport-accept-queue-test branch 2 times, most recently from 61f2639 to 386ef33 Compare December 8, 2022 19:53
@marten-seemann marten-seemann changed the base branch from update-webtransport to master December 8, 2022 20:42
@marten-seemann marten-seemann force-pushed the fix-webtransport-accept-queue-test branch 3 times, most recently from 39a9d9f to 42d7aa0 Compare December 8, 2022 23:47
Copy link
Collaborator

@MarcoPolo MarcoPolo left a comment

Choose a reason for hiding this comment

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

Looks good, but I think would be nice to use receives insteads

p2p/transport/webtransport/transport_test.go Outdated Show resolved Hide resolved
p2p/transport/webtransport/transport_test.go Outdated Show resolved Hide resolved
p2p/transport/webtransport/transport_test.go Outdated Show resolved Hide resolved
@marten-seemann marten-seemann force-pushed the fix-webtransport-accept-queue-test branch from 42d7aa0 to 7a53d58 Compare December 9, 2022 03:00
@marten-seemann marten-seemann force-pushed the fix-webtransport-accept-queue-test branch from 7a53d58 to df1f325 Compare December 9, 2022 05:29
@marten-seemann marten-seemann merged commit 3ecf0b9 into master Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webtransport: flaky TestAcceptQueueFilledUp
2 participants