You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Paul Brett <pa...@twopensource.com> on 2015/07/10 01:11:25 UTC
Review Request 36380: Test cases for performance monitor support of
multiple output versions depending on kernel version.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/
-----------------------------------------------------------
Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
Bugs: MESOS-2834
https://issues.apache.org/jira/browse/MESOS-2834
Repository: mesos
Description
-------
Test cases for performance monitor support of multiple output versions depending on kernel version.
Diffs
-----
src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
Diff: https://reviews.apache.org/r/36380/diff/
Testing
-------
sudo make check
Thanks,
Paul Brett
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/#review91263
-----------------------------------------------------------
src/tests/perf_tests.cpp (line 40)
<https://reviews.apache.org/r/36380/#comment144560>
un-used.
src/tests/perf_tests.cpp (line 76)
<https://reviews.apache.org/r/36380/#comment144559>
looks like this is 'covered' by the new declartion in the for loop.
here and everywhere.
- Chi Zhang
On July 9, 2015, 11:11 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 11:11 p.m.)
>
>
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Test cases for performance monitor support of multiple output versions depending on kernel version.
>
>
> Diffs
> -----
>
> src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
>
> Diff: https://reviews.apache.org/r/36380/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/#review91351
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36378, 36380]
All tests passed.
- Mesos ReviewBot
On July 10, 2015, 8:52 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> -----------------------------------------------------------
>
> (Updated July 10, 2015, 8:52 p.m.)
>
>
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Test cases for performance monitor support of multiple output versions depending on kernel version.
>
>
> Diffs
> -----
>
> src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
>
> Diff: https://reviews.apache.org/r/36380/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/#review91532
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36424, 36378, 36380]
All tests passed.
- Mesos ReviewBot
On July 13, 2015, 9:05 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> -----------------------------------------------------------
>
> (Updated July 13, 2015, 9:05 p.m.)
>
>
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Test cases for performance monitor support of multiple output versions depending on kernel version.
>
>
> Diffs
> -----
>
> src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
>
> Diff: https://reviews.apache.org/r/36380/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/
-----------------------------------------------------------
(Updated July 13, 2015, 9:05 p.m.)
Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
Changes
-------
Incorporate review comments.
Bugs: MESOS-2834
https://issues.apache.org/jira/browse/MESOS-2834
Repository: mesos
Description
-------
Test cases for performance monitor support of multiple output versions depending on kernel version.
Diffs (updated)
-----
src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
Diff: https://reviews.apache.org/r/36380/diff/
Testing
-------
sudo make check
Thanks,
Paul Brett
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Paul Brett <pa...@twopensource.com>.
> On July 10, 2015, 9:43 p.m., Ian Downes wrote:
> > src/tests/perf_tests.cpp, line 159
> > <https://reviews.apache.org/r/36380/diff/2/?file=1008562#file1008562line159>
> >
> > Do you need to keep all the logging of the context on failed expectations after the test has been correctly written? It clutters the code...
If you remove it, you cant tell which test failed so it becomes necessary to re-add it if you ever get a failure.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/#review91346
-----------------------------------------------------------
On July 10, 2015, 8:52 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> -----------------------------------------------------------
>
> (Updated July 10, 2015, 8:52 p.m.)
>
>
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Test cases for performance monitor support of multiple output versions depending on kernel version.
>
>
> Diffs
> -----
>
> src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
>
> Diff: https://reviews.apache.org/r/36380/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/#review91346
-----------------------------------------------------------
src/tests/perf_tests.cpp (line 52)
<https://reviews.apache.org/r/36380/#comment144705>
split args onto separate lines
src/tests/perf_tests.cpp (line 62)
<https://reviews.apache.org/r/36380/#comment144710>
lines? it's 1 or more, how about just calling input?
is debug() a better name for this function?
src/tests/perf_tests.cpp (lines 65 - 69)
<https://reviews.apache.org/r/36380/#comment144709>
use a ternary if here?
```cpp
s << (version.isError() ? "Error:" + version.error()
: version.get());
```
src/tests/perf_tests.cpp (line 104)
<https://reviews.apache.org/r/36380/#comment144706>
I don't think we use this, favoring
```cpp
foreach (const tuple<Version, string>&, input)
```
Is Version hashable? If so, iterating over a Version -> input would be much cleaner
```cpp
foreachpair(const Version& version, const string& input, inputs)
```
Or, just use the string version and parse the string version inside the loop?
```cpp
hashmap<string, string> input {
"2.6.39", "123,cycles\n0.123,task-clock"
}
```
src/tests/perf_tests.cpp (line 157)
<https://reviews.apache.org/r/36380/#comment144714>
Do you need to keep all the logging of the context on failed expectations after the test has been correctly written? It clutters the code...
src/tests/perf_tests.cpp (line 191)
<https://reviews.apache.org/r/36380/#comment144711>
ditto
src/tests/perf_tests.cpp (line 226)
<https://reviews.apache.org/r/36380/#comment144712>
ditto
- Ian Downes
On July 10, 2015, 1:52 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> -----------------------------------------------------------
>
> (Updated July 10, 2015, 1:52 p.m.)
>
>
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Test cases for performance monitor support of multiple output versions depending on kernel version.
>
>
> Diffs
> -----
>
> src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
>
> Diff: https://reviews.apache.org/r/36380/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/
-----------------------------------------------------------
(Updated July 10, 2015, 8:52 p.m.)
Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
Changes
-------
Corrected kernel version number for introduction of change. Added error diagnostics.
Bugs: MESOS-2834
https://issues.apache.org/jira/browse/MESOS-2834
Repository: mesos
Description
-------
Test cases for performance monitor support of multiple output versions depending on kernel version.
Diffs (updated)
-----
src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
Diff: https://reviews.apache.org/r/36380/diff/
Testing
-------
sudo make check
Thanks,
Paul Brett
Re: Review Request 36380: Test cases for performance monitor support
of multiple output versions depending on kernel version.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36380/#review91246
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36378, 36380]
All tests passed.
- Mesos ReviewBot
On July 9, 2015, 11:11 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36380/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 11:11 p.m.)
>
>
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: MESOS-2834
> https://issues.apache.org/jira/browse/MESOS-2834
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Test cases for performance monitor support of multiple output versions depending on kernel version.
>
>
> Diffs
> -----
>
> src/tests/perf_tests.cpp 281eed0094faead67dc7f84df6407686aae88b01
>
> Diff: https://reviews.apache.org/r/36380/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Paul Brett
>
>