Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

remove XCTest expectations with timeouts #1539

Merged
merged 2 commits into from
Jun 2, 2020
Merged

Conversation

weissi
Copy link
Member

@weissi weissi commented Jun 2, 2020

Motivation:

The XCTest expectations caused problems when they time out. When they
time out, the execution continues which in the tests in question then
shut down the EventLoop. Shutting down the event loop meant that other
things don't happen which caused the test to hang. The XCTest
expectations however tell us about the failure at the end of the test
(which we never reach).

Modifications:

Remove the XCTest expectations with timeouts.

Result:

Motivation:

The XCTest expectations caused problems when they time out. When they
time out, the execution continues which in the tests in question then
shut down the EventLoop. Shutting down the event loop meant that other
things don't happen which caused the test to hang. The XCTest
expectations however tell us about the failure at the end of the test
(which we never reach).

Modifications:

Remove the XCTest expectations with timeouts.

Result:

Even slow CI should pass this test correctly.
@weissi weissi requested a review from Lukasa June 2, 2020 12:16
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Good catch

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jun 2, 2020
@Lukasa Lukasa added this to the 2.18.0 milestone Jun 2, 2020
@Lukasa Lukasa merged commit def2f0e into apple:master Jun 2, 2020
@weissi weissi deleted the jw-exp branch June 2, 2020 13:48
pull bot pushed a commit to scope-demo/swift-nio that referenced this pull request Jun 2, 2020
Motivation:

The XCTest expectations caused problems when they time out. When they
time out, the execution continues which in the tests in question then
shut down the EventLoop. Shutting down the event loop meant that other
things don't happen which caused the test to hang. The XCTest
expectations however tell us about the failure at the end of the test
(which we never reach).

Modifications:

Remove the XCTest expectations with timeouts.

Result:

Even slow CI should pass this test correctly.

Co-authored-by: Cory Benfield <lukasa@apple.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky test: EventLoopTest.testScheduleRepeatedTaskCancelFromDifferentThread
2 participants