-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
Sprinkle @slow
on slow tests
#3215
Conversation
2 seconds sounds unnecessarily slow, even if it's only CI. It'd be nice if we could lower the timeout somehow, or cancel it if it hasn't connected in a reasonable time. Though the former sounds difficult if there's no official API, and the latter would change what the test is actually testing for. |
Based on the link in https://stackoverflow.com/a/54599972;
It seems it only actually takes 1s, so we must be trying two ports (ipv4 and ipv6?). Setting registry values doesn't seem like something a test suite should do locally. |
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.
Looks pretty good, agree with jakkdl's comment as well though
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3215 +/- ##
=================================================
Coverage 100.00000% 100.00000%
=================================================
Files 124 124
Lines 18792 20902 +2110
Branches 1268 1641 +373
=================================================
+ Hits 18792 20902 +2110
|
Fixes #3206.
The main thing is that it turns out that waiting for a socket to fail to connect takes up to 2 seconds on Windows -- so tests that rely on that should be marked
@slow
.This approximately halves local test time on Windows.