Re: Review Request 66961: Windows: Ported sendfile_tests.cpp.

2018-05-23 Thread Andrew Schwartzmeyer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66961/#review203697
---


Ship it!




Awesome!


3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 68-73 (patched)


We're going to want to generalize this and my `struct Closer` soon-ish. 
Maybe when we make `int_fd` on Linux an RAII class...


- Andrew Schwartzmeyer


On May 22, 2018, 3:54 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66961/
> ---
> 
> (Updated May 22, 2018, 3:54 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-3444 and MESOS-8681
> https://issues.apache.org/jira/browse/MESOS-3444
> 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 
> 43c5e873a3a3b8f79f0f888a450e186c6123d938 
>   3rdparty/stout/tests/os/sendfile_tests.cpp 
> 05966ae067ae3972598da3370eb16fdce5736c21 
> 
> 
> Diff: https://reviews.apache.org/r/66961/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66961: Windows: Ported sendfile_tests.cpp.

2018-05-22 Thread Akash Gupta 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66961/
---

(Updated May 22, 2018, 10:54 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


Changes
---

Addressed feedback.


Bugs: MESOS-3444 and MESOS-8681
https://issues.apache.org/jira/browse/MESOS-3444
https://issues.apache.org/jira/browse/MESOS-8681


Repository: mesos


Description (updated)
---

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 (updated)
-

  3rdparty/stout/tests/CMakeLists.txt 43c5e873a3a3b8f79f0f888a450e186c6123d938 
  3rdparty/stout/tests/os/sendfile_tests.cpp 
05966ae067ae3972598da3370eb16fdce5736c21 


Diff: https://reviews.apache.org/r/66961/diff/2/

Changes: https://reviews.apache.org/r/66961/diff/1-2/


Testing
---


Thanks,

Akash Gupta

Re: Review Request 66961: Windows: Ported sendfile_tests.cpp.

2018-05-07 Thread Akash Gupta 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66961/
---

(Updated May 7, 2018, 4:15 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


Changes
---

Added another bug.


Bugs: MESOS-3444 and MESOS-8681
https://issues.apache.org/jira/browse/MESOS-3444
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

Re: Review Request 66961: Windows: Ported sendfile_tests.cpp.

2018-05-07 Thread Andrew Schwartzmeyer 

---
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)


Close server?



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 86 (patched)


Close server?



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 92 (patched)


Close server?



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 97 (patched)


Close server?



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 106-109 (patched)


Just make a `WindowsSocketError`, check its `.code`, and return it.



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 117-120 (patched)


Ditto.



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 123 (patched)


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)


You can just use `sandbox` instead of `os::getcwd()`.



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 201 (patched)


`s/We sending/We are sending`



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 207 (patched)


// NOTE: A pending operation is represented by `None()`.



3rdparty/stout/tests/os/sendfile_tests.cpp
Lines 215-217 (patched)


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)


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
> 
>

Review Request 66961: Windows: Ported sendfile_tests.cpp.

2018-05-04 Thread Akash Gupta 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66961/
---

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


Bugs: MESOS-8674
https://issues.apache.org/jira/browse/MESOS-8674


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