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

testConnect() declared a custom Socket but never used it. #331

Merged
merged 1 commit into from
Apr 18, 2018

Conversation

normanmaurer
Copy link
Member

Motivation:

testConnect() should use its custom Socket implementation.

Modifications:

Make the test use the custom Socket

Result:

Correct test

@normanmaurer normanmaurer requested review from weissi and Lukasa April 18, 2018 13:25
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.

@normanmaurer
Copy link
Member Author

I think the test failure is actually related to the same problem as @Lukasa stated here: #330 (comment) (as we also do register first and then call connect)

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

👍

Motivation:

testConnect() should use its custom Socket implementation.

Modifications:

Make the test use the custom Socket

Result:

Correct test
@normanmaurer normanmaurer merged commit 56bb5f4 into apple:master Apr 18, 2018
@normanmaurer normanmaurer added this to the 1.5.0 milestone Apr 18, 2018
@normanmaurer normanmaurer deleted the fix_test_connect branch April 18, 2018 19:16
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Apr 19, 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