-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58706/#review172988
-----------------------------------------------------------
Ship it!
3rdparty/stout/tests/version_tests.cpp
Lines 58-63 (original), 70-82 (patched)
<https://reviews.apache.org/r/58706/#comment246047>
We could simplify this a bit:
```
Try<Version> actual = Version::parse(input);
ASSERT_SOME_EQ(std::get<0>(expected), actual)
<< "Incorrect parse of input '" << input << "'";
EXPECT_EQ(std::get<1>(expected), stringify(actual.get()))
<< "Incorrect parse of input '" << input << "'";
```
3rdparty/stout/tests/version_tests.cpp
Lines 86 (patched)
<https://reviews.apache.org/r/58706/#comment246036>
This reads to me that malformed input strings can be parsed, maybe re-phrase? Also would
be nice to use consistent terminology ("malformed" vs "invalid") here
3rdparty/stout/tests/version_tests.cpp
Line 64 (original), 86-106 (patched)
<https://reviews.apache.org/r/58706/#comment246039>
Naming wise, how about:
```
const vector<string> inputs = {...};
foreach (const string& input, inputs) {
Try<Version> parse = Version::parse(input);
// Or:
Try<Version> version = Version::parse(input);
...
}
```
Actual seems a bit out of place in this one given there's no "expected" to go along with
it.
- Benjamin Mahler
On April 25, 2017, 8:03 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58706/
> -----------------------------------------------------------
>
> (Updated April 25, 2017, 8:03 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Kapil Arya.
>
>
> Bugs: MESOS-1987
> https://issues.apache.org/jira/browse/MESOS-1987
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Switch to table-based test cases, validate that version parsing produces
> the expected results more precisely.
>
>
> Diffs
> -----
>
> 3rdparty/stout/tests/version_tests.cpp 724ed2292fdd3c5f4c98facf82260078b66a0e97
>
>
> Diff: https://reviews.apache.org/r/58706/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Neil Conway
>
>
|