Skip to content

Attempt to unbreak test timeouts in Windows. #4622

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 1 commit into from
Aug 10, 2022

Conversation

3405691582
Copy link
Member

On Windows, __CFTSRRate is in units of 100 ns. To get a CFTimeInterval
from mach_absolute_time therefore, we should divide by 1.0E7, not 1.0E9.
But this suggests we should be dividing by __CFTSRRate anyway.

(Currently trying to test this now, so making this a draft pr for now to share since #3004 may be causing trouble for Windows.)

@3405691582
Copy link
Member Author

cc @compnerd

@3405691582
Copy link
Member Author

In fact, I don't actually think doing this is correct, I think just dividing by the various units is more correct. The kicker is, of course, clock_getres is no longer returning funny clock resolutions on OpenBSD 7.1 and so I can't strictly reproduce the problem. Instead, I think a separate Windows branch that divides by 1.0E7 as a bare constant might make more sense.

This shouldn't bite OpenBSD 7.1 and may fix regressions on Windows. The
underlying time code is still a little suspect, but this might be the
safest thing to do right now.
@3405691582
Copy link
Member Author

3405691582 commented Aug 10, 2022

What's still confusing me is that while the clock resolution on Windows is different, "fixing" the incommensurability of TSR units and nanoseconds actually broke Windows. To try to avoid spending too much time loading this stuff back into mental cache to try and untangle this, the safest course of action may be to just revert the attempt to fix #4220.

This shouldn't bite OpenBSD now clock_getres is returning 1 like TARGET_OS_LINUX and should probably fix Windows since it was working before.

@compnerd, please try picking this and seeing if the test timeout troubles you no longer.

@3405691582 3405691582 changed the title Use __CFTSRRate in __CFNanosecondsToTSR. Attempt to unbreak test timeouts in Windows. Aug 10, 2022
@compnerd
Copy link
Member

Triggered a build, should hopefully know in 90m.

@compnerd
Copy link
Member

Okay, this does seem to restore the Windows builds.

@compnerd compnerd marked this pull request as ready for review August 10, 2022 02:16
@compnerd
Copy link
Member

@swift-ci please test

@3405691582
Copy link
Member Author

3405691582 commented Aug 10, 2022

Just finally confirmed the Foundation RunLoop tests pass on OpenBSD, so this is good to go when ready:

$ ./bin/TestFoundation TestFoundation.TestRunLoop
...
Test Suite 'Selected tests' started at 2022-08-09 23:31:52.838
Test Suite 'TestRunLoop' started at 2022-08-09 23:31:52.844
Test Case 'TestRunLoop.test_constants' started at 2022-08-09 23:31:52.844
Test Case 'TestRunLoop.test_constants' passed (0.0 seconds)
Test Case 'TestRunLoop.test_runLoopInit' started at 2022-08-09 23:31:52.844
Test Case 'TestRunLoop.test_runLoopInit' passed (0.001 seconds)
Test Case 'TestRunLoop.test_commonModes' started at 2022-08-09 23:31:52.845
Test Case 'TestRunLoop.test_commonModes' passed (0.004 seconds)
Test Case 'TestRunLoop.test_runLoopRunMode' started at 2022-08-09 23:31:52.849
Test Case 'TestRunLoop.test_runLoopRunMode' passed (0.05 seconds)
Test Case 'TestRunLoop.test_runLoopLimitDate' started at 2022-08-09 23:31:52.900
Test Case 'TestRunLoop.test_runLoopLimitDate' passed (0.0 seconds)
Test Case 'TestRunLoop.test_runLoopPoll' started at 2022-08-09 23:31:52.900
Test Case 'TestRunLoop.test_runLoopPoll' passed (0.0 seconds)
Test Case 'TestRunLoop.test_addingRemovingPorts' started at 2022-08-09 23:31:52.900
Test Case 'TestRunLoop.test_addingRemovingPorts' passed (0.003 seconds)
Test Case 'TestRunLoop.test_mainDispatchQueueCallout' started at 2022-08-09 23:31:52.903
Test Case 'TestRunLoop.test_mainDispatchQueueCallout' passed (2.001 seconds)
Test Suite 'TestRunLoop' passed at 2022-08-09 23:31:54.904
         Executed 8 tests, with 0 failures (0 unexpected) in 2.059 (2.059) seconds
Test Suite 'Selected tests' passed at 2022-08-09 23:31:54.904
         Executed 8 tests, with 0 failures (0 unexpected) in 2.059 (2.059) seconds

@compnerd
Copy link
Member

Thanks!

@compnerd compnerd merged commit cce3d9a into swiftlang:main Aug 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.

2 participants