Skip to content

chore: Update pion/udp and improve parallel/non-parallel tests #7164

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

Merged
merged 5 commits into from
Apr 17, 2023

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Apr 17, 2023

Tests that mix parallel/non-parallel parent tests and subtests will behave unexpectedly, this PR attempts to clean some of these up and does a little restructuring.

Ultimately, don't mix parallel/non-parallel and avoid too much nesting (sub-subtests).

@mafredri
Copy link
Member Author

mafredri commented Apr 17, 2023

After re-enabling port-forward tests, I noticed we've at some point ditched our fork of pion/udp which does not include this fix: pion/udp@ab18689

So I updated the package to the next version (last one before their v2 work).

See: https://github.com/coder/coder/actions/runs/4723449296/jobs/8379423014?pr=7164

@mafredri mafredri force-pushed the mafredri/test-improve-paralleltest-setup branch from 284114b to 67caa6a Compare April 17, 2023 16:30
@@ -21,10 +21,8 @@ import (
"github.com/coder/coder/testutil"
)

//nolint:tparallel,paralleltest // Subtests require setup that must not be done in parallel.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm hoping removing parallel from the parent will allow these to run more stable, so for now let's re-enable these tests.

err = <-errC
require.ErrorIs(t, err, context.Canceled)
})
t.Run(c.name+"_OnePort", func(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This may also contribute to stability, one less test indentation. We probably need to start considering depth of tests and their interactions with each other or limiting depth to parent + subtest, not subsubtest.

@mafredri mafredri marked this pull request as ready for review April 17, 2023 17:17
@mafredri mafredri changed the title test(all): Improve and fix subtests with parallell/nonparallel parents chore: Update pion/udp and improve parallel/non-parallel tests Apr 17, 2023
@mafredri mafredri merged commit bf0fed4 into main Apr 17, 2023
@mafredri mafredri deleted the mafredri/test-improve-paralleltest-setup branch April 17, 2023 17:23
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants