mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 69584: Relaxed matching criteria for test filters.
Date Tue, 18 Dec 2018 19:21:01 GMT


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


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