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

Fix warning during compiling tests. #356

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

normanmaurer
Copy link
Member

Motivation:

When compiling the tests the following warning is printed:

swift-nio/Tests/NIOTests/SelectorTest.swift:361:43: warning: result of call to 'wait()' is unused
}.wait().forEach { return try! $0.wait() } as Void)

Modifications:

Ignore return value to silent the compiler warnings.

Result:

No more warnings.

@normanmaurer normanmaurer requested review from Lukasa and weissi April 25, 2018 18:39
@tomerd
Copy link
Member

tomerd commented Apr 26, 2018

@swift-nio-bot test this please

@@ -358,7 +358,7 @@ class SelectorTest: XCTestCase {
return channel
}
}
}.wait().forEach { return try! $0.wait() } as Void)
}.wait().forEach { _ = try! $0.wait() } as Void)
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than just suppress the return value, why not wrap this in a XCTAssertNoThrow and remove the try!? That way we have .forEach { XCTAssertNoThrow(try $0.wait) }, which would be a bit better.

Copy link
Member Author

Choose a reason for hiding this comment

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

sure why not...

Motivation:

When compiling the tests the following warning is printed:

swift-nio/Tests/NIOTests/SelectorTest.swift:361:43: warning: result of call to 'wait()' is unused
        }.wait().forEach { return try! $0.wait() } as Void)

Modifications:

Use XCTAssertNoThrow to silence the warnings.

Result:

No more warnings.
@normanmaurer normanmaurer force-pushed the fix_selector_test_warning branch from b9598a6 to 077e4e3 Compare April 26, 2018 08:46
@normanmaurer
Copy link
Member Author

@Lukasa addressed

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.

LGTM, thanks!

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Apr 26, 2018
@Lukasa Lukasa added this to the 1.6.0 milestone Apr 26, 2018
@Lukasa Lukasa merged commit 91ada57 into apple:master Apr 26, 2018
# 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.

3 participants