You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/12/06 19:28:27 UTC
Re: Review Request 53803: Added a new libprocess HTTP test.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/
-----------------------------------------------------------
(Updated Dec. 6, 2016, 7:28 p.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Rebase.
Bugs: MESOS-5966
https://issues.apache.org/jira/browse/MESOS-5966
Repository: mesos
Description
-------
This patch adds HTTPTest.SocketEOF to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.
Diffs (updated)
-----
3rdparty/libprocess/src/tests/http_tests.cpp 7f5425742d554b78fd4788c4a453068a604e1f46
Diff: https://reviews.apache.org/r/53803/diff/
Testing
-------
Testing details are at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 53803: Added new libprocess socket tests.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/#review161625
-----------------------------------------------------------
Ship it!
I can tweak the below comments before committing.
3rdparty/libprocess/src/tests/socket_tests.cpp (line 89)
<https://reviews.apache.org/r/53803/#comment232882>
Typo on `Parametrize`
3rdparty/libprocess/src/tests/socket_tests.cpp (lines 133 - 140)
<https://reviews.apache.org/r/53803/#comment232885>
To future proof this code, this macro should (unfortunately) be expanded into:
```
#ifdef USE_SSL_SOCKET
INSTANTIATE_TEST_CASE_P(
Encryption,
NetSocketTest,
::testing::Values(
string("SSL"),
string("Non-SSL")));
#else
INSTANTIATE_TEST_CASE_P(
Encryption,
NetSocketTest,
::testing::Values(
string("Non-SSL")));
#endif // USE_SSL_SOCKET
```
By the looks of it, this test will be ported to Windows, which means that MSVC will refuse to expand the #ifdef inside the test macro.
3rdparty/libprocess/src/tests/socket_tests.cpp (lines 147 - 148)
<https://reviews.apache.org/r/53803/#comment232883>
Let's move this to the upper part of the file.
- Joseph Wu
On Jan. 11, 2017, 4:57 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53803/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 4:57 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-6802
> https://issues.apache.org/jira/browse/MESOS-6802
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds NetSocketTest.EOFBeforeRecv and
> NetSocketTest.EOFAfterRecv to verify that EOFs are
> reliably received whether or not there is a pending recv()
> request at the time the EOF is received.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/tests/socket_tests.cpp 44c3c9adc39702dd598aa0105088517df601bbda
>
> Diff: https://reviews.apache.org/r/53803/diff/
>
>
> Testing
> -------
>
> Testing details are at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 53803: Added new libprocess socket tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/
-----------------------------------------------------------
(Updated Jan. 12, 2017, 12:57 a.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Bugs: MESOS-6802
https://issues.apache.org/jira/browse/MESOS-6802
Repository: mesos
Description
-------
This patch adds NetSocketTest.EOFBeforeRecv and
NetSocketTest.EOFAfterRecv to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.
Diffs
-----
3rdparty/libprocess/src/tests/socket_tests.cpp 44c3c9adc39702dd598aa0105088517df601bbda
Diff: https://reviews.apache.org/r/53803/diff/
Testing
-------
Testing details are at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 53803: Added new libprocess socket tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/
-----------------------------------------------------------
(Updated Jan. 11, 2017, 12:52 a.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Bugs: MESOS-6802
https://issues.apache.org/jira/browse/MESOS-6802
Repository: mesos
Description
-------
This patch adds NetSocketTest.EOFBeforeRecv and
NetSocketTest.EOFAfterRecv to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.
Diffs
-----
3rdparty/libprocess/src/tests/socket_tests.cpp 44c3c9adc39702dd598aa0105088517df601bbda
Diff: https://reviews.apache.org/r/53803/diff/
Testing
-------
Testing details are at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 53803: Added new libprocess socket tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/
-----------------------------------------------------------
(Updated Dec. 15, 2016, 5:54 p.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Bugs: MESOS-6802
https://issues.apache.org/jira/browse/MESOS-6802
Repository: mesos
Description
-------
This patch adds NetSocketTest.EOFBeforeRecv and
NetSocketTest.EOFAfterRecv to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.
Diffs
-----
3rdparty/libprocess/src/tests/socket_tests.cpp 44c3c9adc39702dd598aa0105088517df601bbda
Diff: https://reviews.apache.org/r/53803/diff/
Testing
-------
Testing details are at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 53803: Added new libprocess socket tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/
-----------------------------------------------------------
(Updated Dec. 10, 2016, 1:31 a.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Moved the tests to 'socket_tests.cpp'.
Summary (updated)
-----------------
Added new libprocess socket tests.
Bugs: MESOS-5966
https://issues.apache.org/jira/browse/MESOS-5966
Repository: mesos
Description (updated)
-------
This patch adds NetSocketTest.EOFBeforeRecv and
NetSocketTest.EOFAfterRecv to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.
Diffs (updated)
-----
3rdparty/libprocess/src/tests/socket_tests.cpp 44c3c9adc39702dd598aa0105088517df601bbda
Diff: https://reviews.apache.org/r/53803/diff/
Testing
-------
Testing details are at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 53803: Added a new libprocess HTTP test.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/
-----------------------------------------------------------
(Updated Dec. 8, 2016, 9:42 p.m.)
Review request for mesos, Benjamin Mahler and Joseph Wu.
Changes
-------
Addressed Joseph's comments.
Bugs: MESOS-5966
https://issues.apache.org/jira/browse/MESOS-5966
Repository: mesos
Description
-------
This patch adds HTTPTest.SocketEOF to verify that EOFs are
reliably received whether or not there is a pending recv()
request at the time the EOF is received.
Diffs (updated)
-----
3rdparty/libprocess/src/tests/http_tests.cpp f4c5dd6dd175b4b91a9517a280a0013fd588752f
Diff: https://reviews.apache.org/r/53803/diff/
Testing
-------
Testing details are at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 53803: Added a new libprocess HTTP test.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53803/#review158485
-----------------------------------------------------------
3rdparty/libprocess/src/tests/http_tests.cpp (lines 15 - 16)
<https://reviews.apache.org/r/53803/#comment229256>
Is this header still required?
3rdparty/libprocess/src/tests/http_tests.cpp (lines 222 - 224)
<https://reviews.apache.org/r/53803/#comment229253>
Why are both of these inner scopes in the same test? They could be entirely separate tests, right?
3rdparty/libprocess/src/tests/http_tests.cpp (line 238)
<https://reviews.apache.org/r/53803/#comment229258>
This will run into an error on Windows, as the `server_address` is the (invalid) 0.0.0.0 IP.
See: https://github.com/apache/mesos/blob/1350f946a3aa12e9b2b66720e81b5a4ba4c4d90d/3rdparty/libprocess/src/tests/http_tests.cpp#L1784-L1794
Ditto in the second block.
- Joseph Wu
On Dec. 6, 2016, 11:28 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53803/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2016, 11:28 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Joseph Wu.
>
>
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds HTTPTest.SocketEOF to verify that EOFs are
> reliably received whether or not there is a pending recv()
> request at the time the EOF is received.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/tests/http_tests.cpp 7f5425742d554b78fd4788c4a453068a604e1f46
>
> Diff: https://reviews.apache.org/r/53803/diff/
>
>
> Testing
> -------
>
> Testing details are at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>