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

Run Quinn tests in CI #6719

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Run Quinn tests in CI #6719

merged 2 commits into from
Jul 25, 2024

Conversation

djc
Copy link
Contributor

@djc djc commented Jul 24, 2024

Motivation

Improve testing for timers (which caused a regression, #6714) by testing Quinn in CI.

Fixes #6717.

@djc djc requested a review from Darksonn July 24, 2024 13:18
@djc djc changed the title Quinn ci Run Quinn tests in CI Jul 24, 2024
@djc djc force-pushed the quinn-ci branch 2 times, most recently from 1edfabd to 575892c Compare July 24, 2024 13:36
@djc
Copy link
Contributor Author

djc commented Jul 24, 2024

Not sure how/why warnings are being denied, probably makes sense to allow them for these external crates?

@Darksonn Darksonn added the A-ci Area: The continuous integration setup label Jul 24, 2024
@Darksonn
Copy link
Contributor

You probably need to override RUST_FLAGS.

RUSTFLAGS: -Dwarnings

@Darksonn
Copy link
Contributor

Any chance you could try pushing a commit that undoes the revert to verify that this would have caught the regression?

@github-actions github-actions bot added the R-loom-time-driver Run loom time driver tests on this PR label Jul 25, 2024
@djc
Copy link
Contributor Author

djc commented Jul 25, 2024

I used

djc-2021 quinn-ci tokio $ git cherry-pick 8480a180e6ffdd0ec0ec213a9bebcda2445fc541

and that resulted in failure.

So I think this is working as intended, and I'll remove the cherry-picked commit again.

@github-actions github-actions bot removed the R-loom-time-driver Run loom time driver tests on this PR label Jul 25, 2024
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Awesome, thank you. Verified that this would have caught the regression.

.github/workflows/ci.yml Show resolved Hide resolved
@Darksonn Darksonn merged commit 3297052 into master Jul 25, 2024
80 checks passed
@Darksonn Darksonn deleted the quinn-ci branch July 25, 2024 09:37
@Darksonn
Copy link
Contributor

Thanks!

@djc
Copy link
Contributor Author

djc commented Jul 25, 2024

Thank you for all your work on Tokio!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Area: The continuous integration setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve testing of timers
2 participants