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



Not quite sure about the false positive issue:

"This change is also safe regarding false positives, since our
naming conventions forbid the matched strings from appearing
naturally in any test name."

Ah, I wasn't aware of the convention. But I feel it is easy to slip?
For example, after this patch, would we match 
"ROOT_DOCKER_CGROUPS_CFS_CgroupsEnableCFS" if I remove the "CFS_" tag?

If that is the case, I would prefer to keep the original matching criteria and 
fix the test name. It is easy to
spot a test that ran (but not supposed to) than finding out a silent missing 
test.

- Meng Zhu


On Dec. 18, 2018, 6:59 a.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69584/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2018, 6:59 a.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed the test filters to remove the required trailing
> underscore, e.g. the `BenchmarkFilter` would now match all test names
> containing the string `BENCHMARK`, as opposed to `BENCHMARK_`.
> 
> The latter filter would fail to match test names like
> `HierarchicalAllocations_BENCHMARK.MultiFrameworkAllocations`,
> contrary to the test authors assumptions.
> 
> This change is also safe regarding false positives, since our
> naming conventions forbid the matched strings from appearing
> naturally in any test name.
> 
> 
> Diffs
> -----
> 
>   src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 
> 
> 
> Diff: https://reviews.apache.org/r/69584/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Reply via email to