----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66961/#review202589 -----------------------------------------------------------
3rdparty/stout/tests/os/sendfile_tests.cpp Lines 82 (patched) <https://reviews.apache.org/r/66961/#comment284462> Close server? 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 86 (patched) <https://reviews.apache.org/r/66961/#comment284463> Close server? 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 92 (patched) <https://reviews.apache.org/r/66961/#comment284464> Close server? 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 97 (patched) <https://reviews.apache.org/r/66961/#comment284465> Close server? 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 106-109 (patched) <https://reviews.apache.org/r/66961/#comment284467> Just make a `WindowsSocketError`, check its `.code`, and return it. 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 117-120 (patched) <https://reviews.apache.org/r/66961/#comment284466> Ditto. 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 123 (patched) <https://reviews.apache.org/r/66961/#comment284468> Should this just get wrapped into something that will `os::close` when it leaves scope? There's a lot of manually handling its destruction in this function. 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 188-189 (patched) <https://reviews.apache.org/r/66961/#comment284469> You can just use `sandbox` instead of `os::getcwd()`. 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 201 (patched) <https://reviews.apache.org/r/66961/#comment284470> `s/We sending/We are sending` 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 207 (patched) <https://reviews.apache.org/r/66961/#comment284471> // NOTE: A pending operation is represented by `None()`. 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 215-217 (patched) <https://reviews.apache.org/r/66961/#comment284473> I'm starting to change my mind on the "be consistent with Windows `TRUE/FALSE` policy"... This would be so much cleaner as `EXPECT_TRUE(...)`. 3rdparty/stout/tests/os/sendfile_tests.cpp Lines 219-221 (patched) <https://reviews.apache.org/r/66961/#comment284472> These (and the other non-fatal ones) should be `EXPECT` instead of `ASSERT`. - Andrew Schwartzmeyer On May 4, 2018, 10:17 a.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66961/ > ----------------------------------------------------------- > > (Updated May 4, 2018, 10:17 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, > John Kordich, Joseph Wu, and Radhika Jandhyala. > > > Bugs: MESOS-8681 > https://issues.apache.org/jira/browse/MESOS-8681 > > > Repository: mesos > > > Description > ------- > > The sendfile tests were using `socketpair`, but the rest of the tests > were crossplatform. By providing a Windows `socketpair` implementations, > the tests are now ported. > > > Diffs > ----- > > 3rdparty/stout/tests/CMakeLists.txt > 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec > 3rdparty/stout/tests/os/sendfile_tests.cpp > 05966ae067ae3972598da3370eb16fdce5736c21 > > > Diff: https://reviews.apache.org/r/66961/diff/1/ > > > Testing > ------- > > > Thanks, > > Akash Gupta > >
