> On Dec. 18, 2018, 10:44 a.m., Meng Zhu wrote: > > 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.
I agree about the false positive case. We should just update the specific BENCHMARK test to properly filter it. I don't know if we codify our test naming style anywhere at the moment. However, our naming convention is to put any of the test filters at the beginning of a test name, rather than the end. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69584/#review211410 ----------------------------------------------------------- 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 > >
