mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Budnik <abud...@mesosphere.com>
Subject Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.
Date Fri, 07 Jul 2017 15:05:07 GMT


> On July 7, 2017, 1:15 p.m., Alexander Rojas wrote:
> > src/linux/perf.cpp
> > Lines 243 (patched)
> > <https://reviews.apache.org/r/60397/diff/3/?file=1768325#file1768325line243>
> >
> >     Why this command? two lines later `perf::version()` is called. Isn't there a
way to merge this with the `perf::version()` call?

`perf::version()` also used in tests:
```cpp
// Returns the detected perf version. Exposed for testing.
process::Future<Version> version();
```
If I move this code to `perf::version()`, then its failure might be interpreted in multiple
ways:
1. `perf --version` failed => perf isn’t installed or coredumped
2. `parseVersion()` failed
`PerfTest.Version` test checks whether we are able to parse perf version. If perf isn’t
installed, the test shall pass. Quote from James:
“This drops the test that verifies we can actually parse perf versions that are found in
the wild. That test is necessary to catch when new systems play games with the perf version;
otherwise we would just detect that the perf version isn’t supported and it would be hard
to diagnose why not.”


- Andrei


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


On July 3, 2017, 10:57 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 10:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
>     https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -----
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> -------
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


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