Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Port gaea tests #1070

Merged
merged 16 commits into from
Sep 12, 2019
Merged

Port gaea tests #1070

merged 16 commits into from
Sep 12, 2019

Conversation

Thomasdezeeuw
Copy link
Collaborator

Still have to port the tests for TcpStream and TcpListener.

@Thomasdezeeuw
Add tests for the registration process
bd67037 
Check that the event::Source implementation is correctly called.
@Thomasdezeeuw
Port tests for UdpSocket from Gaea
2a91ee0 
There is some overlap in what is tested now however.
@Thomasdezeeuw
Copy link
Collaborator Author

Note that there is some overlap in the tests for UdpSocket now, if I have some time I'll remove the unneeded tests.

carllerche
carllerche approved these changes Aug 29, 2019
View reviewed changes
Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

👍 cheers.

Thomasdezeeuw
Thomasdezeeuw commented Aug 31, 2019
View reviewed changes
tests/tcp_listener.rs

// Expect the stream to be non-blocking.
let mut buf = [0; 20];
assert_would_block(stream.read(&mut buf));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@carllerche here's a test that ensure the stream to be non-blocking, per what you mentioned on Gitter.

@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Aug 31, 2019
Closed
@Thomasdezeeuw
Set accepted TcpStreams in non-blocking mode
a7e531e
@Thomasdezeeuw
Disable TcpListener deregister test on Windows
d88f51a 
Doesn't work, see issue tokio-rs#1070.
@Thomasdezeeuw
Make TcpListener::try_clone tests more robust
b20377a 
Before it could be that only one of the connections would be ready to be
accepted, result in would-block errors.
@Thomasdezeeuw
Copy link
Collaborator Author

It seems Linux and macOS handles shutting down the reading side differently. Linux still allows the remaining buffer to be read, while macOS doesn't.

@carllerche
Copy link
Member

Shutting down the reading side is pretty much unspecified and has no meaning at the TCP level. I wouldn’t bother testing it.

@Thomasdezeeuw
Loosen TcpStream shutdown reading side test
ac477b4 
What happens is different on each OS, for example on Linux we can still
read but on macOS and FreeBSD we can't.
@Thomasdezeeuw
Ignore connection reset error in TcpStream test
9a49628 
In a attempt to fix the TcpStream shutdown_read test.
@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Aug 31, 2019
Closed
@Thomasdezeeuw
Disable TcpListener::try_clone tests on Windows
e99caf3 
They both fails, be it's unknown to me why. It needs further
investigation for which I've opened tokio-rs#1074.
@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Aug 31, 2019
Closed
@carllerche
Copy link
Member

Ideally we can get this merged sooner than later as it has the non-blocking fixes.

@Thomasdezeeuw
Copy link
Collaborator Author

@carllerche sorry for the delay, I haven't had much time recently to work on this. As for the failures; there are a number of network issues which should resolves itself. The only actual failures are on Windows the tcp_stream_ipv4 and tcp_stream_ipv6 tests fails because they don't receive an expected event here: https://github.com/tokio-rs/mio/pull/1070/files#diff-c448a2bd877d650a0a8a8c0cdb0dba95R50. This looks like a bug in the Windows implementation to me.

I could ignore the tests for now on Windows so we can merge this and open an issue for it to fix it later.

@Thomasdezeeuw
Fix TcpStream's shutdown_both tests
d479097 
On Windows the error is ConnectionAborted rather then BrokenPipe.
@Thomasdezeeuw
Copy link
Collaborator Author

I think something strange is going in the nodelay test failure: https://dev.azure.com/tokio-rs/Tokio/_build/results?buildId=2295&view=logs&jobId=429a383f-e486-57ac-3982-9ac65641a32a&taskId=1f926ffb-0ec7-5f9b-ab05-76fbb04597a8&lineStart=168&lineEnd=168&colStart=1&colEnd=7, error message: panicked at 'called Result::unwrap()on anErr value: Os { code: 10022, kind: InvalidInput, message: "An invalid argument was supplied." }'. It passed in the previous commits and no changes were made, further more the argument can't be invalid as only true or false are possible and std lib takes cares of the rest.

@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Sep 7, 2019
Closed
@Thomasdezeeuw
Ignore TcpStream smoke tests on Windows
8c7af5f 
Issue tokio-rs#1078 is opened to investigate why they fail on Windows only.
@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Sep 7, 2019
Closed
@Thomasdezeeuw
Ignore TcpStream nodelay test on Windows
4f2913d 
Issue tokio-rs#1079 is opened to investigate why it fails on Windows only.
@Thomasdezeeuw
Copy link
Collaborator Author

I've opened some issues because some tests are failing on Windows, #1073, #1074, #1078 and #1079, in an attempt to get this merged.

@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Sep 8, 2019
Closed
@Thomasdezeeuw
Ignore some UdpSocket tests on Windows
112a425 
Issues tokio-rs#1080 is opened to investigate why.
@Thomasdezeeuw
Copy link
Collaborator Author

@carllerche CI finally green, I've opened a number of issue to investigate why the tests fail on Windows, but I currently don't have the time to investigate. In any case the is ready for a review.

@Thomasdezeeuw Thomasdezeeuw marked this pull request as ready for review September 8, 2019 11:06
@carllerche carllerche mentioned this pull request Sep 12, 2019
Closed
16 tasks
@carllerche
Copy link
Member

Looks great! we can iterate more on master in follow up PRs.

@Thomasdezeeuw Thomasdezeeuw merged commit 3517758 into tokio-rs:master Sep 12, 2019
Thomasdezeeuw added a commit that referenced this pull request Sep 12, 2019
@Thomasdezeeuw
Disable TcpListener deregister test on Windows
414ad2b 
Doesn't work, see issue #1070.
@Thomasdezeeuw Thomasdezeeuw deleted the port-gaea-tests branch September 12, 2019 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carllerche carllerche carllerche approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Thomasdezeeuw @carllerche