mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 69584: Relaxed matching criteria for test filters.
Date Tue, 18 Dec 2018 18:44:07 GMT

-----------------------------------------------------------
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
> 
>


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