Skip to content

Tests: add a workaround for Windows #334

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
Nov 6, 2021
Merged

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Nov 5, 2021

The Windows test regularly fails if the stdout is not flushed on
completion. This increases the stability of the tests on Windows
(though there is another async related issue lurking that causes a
sporadic failure).

@compnerd
Copy link
Member Author

compnerd commented Nov 5, 2021

@swift-ci please test

@@ -56,7 +60,11 @@ class AsyncAwaitTests: XCTestCase {

override func setUp() async throws {}

override func tearDown() async throws {}
override func tearDown() async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, although I wonder if it would be ever-so-slightly more robust to place this flush in an override of the class-level tearDown method rather than this instance one? The class one isn't async but it runs later, after all tests and their tearDown methods complete, so it would theoretically be more inclusive in case there were stdout generated after an instance tearDown. I.e.

Suggested change
override func tearDown() async throws {
override class func tearDown() {

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 think that I am going to disable the test on Windows instead. I noticed that the runtime was triggering asserts, and I think that is the core issue and that the flush might be a red herring. Sorry about the change of approach.

@compnerd
Copy link
Member Author

compnerd commented Nov 5, 2021

Hmm, when I went to test this, I happened to notice an assertion in the runtime. I think that it might be that we should just temporarily disable this one test on Windows. I can try to look into the runtime assertion subsequently.

Disable this test on Windows as this is exposing a latent UB in the
Concurrency runtime.  It seems better to safely disable this test on
Windows for the time being until the runtime is fixed.
@compnerd
Copy link
Member Author

compnerd commented Nov 5, 2021

@swift-ci please test

@compnerd compnerd merged commit 3a4fa9f into swiftlang:main Nov 6, 2021
@compnerd compnerd deleted the flush branch November 6, 2021 03:09
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