mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.
Date Wed, 02 Sep 2015 22:47:34 GMT

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


Thinking about this further, the test parameteratization here seems to be a bit of a mess,
could we instead just create lists of valid / invalid inputs within a test and loop over them?
I don't think the test parameterization here is worth the complexity.


src/tests/containerizer/perf_tests.cpp (line 51)
<https://reviews.apache.org/r/37466/#comment153537>

    Why not just Events? This is testing both valid and invalid events.
    
    Also, can you pull this out into a separate patch?



src/tests/containerizer/perf_tests.cpp (line 67)
<https://reviews.apache.org/r/37466/#comment153545>

    Why is this called ParseTypes and the one below called ParseCgroups? They both seem to
parse cgroup-based perf output, so I'm a bit confused.



src/tests/containerizer/perf_tests.cpp (lines 85 - 100)
<https://reviews.apache.org/r/37466/#comment153541>

    Please instantiate right below the class as it makes it clearer when there are many tests
under the test fixture (it's also what we do in all the other tests).
    
    Ditto for the rest of these.



src/tests/containerizer/perf_tests.cpp (lines 223 - 233)
<https://reviews.apache.org/r/37466/#comment153544>

    What's with the inconsistent newlines here?



src/tests/containerizer/perf_tests.cpp (line 253)
<https://reviews.apache.org/r/37466/#comment153543>

    I think you meant to split the string literal on the newline like you did in most cases,
can you do a sweep?


- Ben Mahler


On Sept. 2, 2015, 9:13 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37466/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2015, 9:13 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-2834
>     https://issues.apache.org/jira/browse/MESOS-2834
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Update perf tests to including testing the supported perf output formats.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/perf_tests.cpp 8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 
> 
> Diff: https://reviews.apache.org/r/37466/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message