> On April 12, 2016, 3:09 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/strerror_tests.cpp, lines 22-23
> > <https://reviews.apache.org/r/46009/diff/1/?file=1339067#file1339067line22>
> >
> >     This seems generally useful?
> >     I think I've even seen it in other reviews?
> >     Why is it burried in a test?
> 
> Alex Clemmer wrote:
>     We are doing this to stay consistent with Neil Conway's pass over the 
> tests to use `std::string`. For the general codebase, maybe it's useful, and 
> maybe it's not, I'm not sure.
> 
> Joris Van Remoortere wrote:
>     Sorry I must have messed up the selection highlight.
>     I meant this: `const string errorMessage(int err)`.

We already have code like this in `WindowsError`, and slightly different code 
like this in `os::hstrerror`.

The thinking was, the tests below are calling `::strerror` and comparing them 
to `os::strerror`, and what we'd really like to do is just call 
`::FormatMessage` straight up, but it's a bit too verbose to embed directly in 
the test. Hence, I wrote a small helper function.

Let me know what else you would like to see here.


- Alex


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


On April 13, 2016, 10:05 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46009/
> -----------------------------------------------------------
> 
> (Updated April 13, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Stout:[1/2] Added simple tests for `os::` functions.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 400c6dc451602926f93b22713af8c66d7ca59ca6 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> c9d331df2f4496183b5734d2434413f68b9c1b4b 
>   3rdparty/libprocess/3rdparty/stout/tests/os/env_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os/strerror_tests.cpp 
> d5a96ad6b8b1c09b4f016e0c8e3e5c5672b55ef3 
> 
> Diff: https://reviews.apache.org/r/46009/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Reply via email to