network_service_content_browsertests flakily failed with test runner error |
||
Issue descriptiontask: https://chromium-swarm.appspot.com/task?id=40e40f67d9921610&refresh=10&show_raw=1 CL: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-kitkat-arm-rel/115389 """ test_runner.py gtest: error: argument -f/--test-filter/--gtest_filter/--gtest-filter: not allowed with argument --gtest-filter-file/--test-launcher-filter-file """
,
Nov 1
This is that same CL :/
,
Nov 1
Why are these mutually exclusive? Is there a particular reason? I think they both should work, or we could somehow remove the filter file argument on the retry.
,
Nov 1
I see many more examples of "test-launcher-filter-file" in test_suites.pyl. I would expect any 'retry with patch' or 'retry without patch' step to fail when run on a test suite that also sets "test-launcher-filter-file".
,
Nov 1
Just because muxing them in https://codesearch.chromium.org/chromium/src/build/android/pylib/utils/test_filter.py?rcl=e52da1b8552c9a23e332dfc63b5cb480f3c6966f&l=69 isn't implemented; now that it's necessary, there's no particular reason not to implement it.
,
Nov 1
I think that --gtest_filter should override --test-launcher-filter-file, which would also be very easy to implement. Objections?
,
Nov 1
#6 SGTM
,
Nov 1
That doesn't seem particularly intuitive?
,
Nov 1
If a user [or script] is specifying --gtest_filter, then they're indicating that they want a very specific suite of tests to run. On the other hand, --test-launcher-filter-file appears to be used to generically filter out a large number of tests. Most of the time, I expect there to be no conflict between --gtest_filter and --test-launcher-filter-file. E.g. in the case of recipes, in order for a test to fail, it must not have been filtered by test-launcher-filter-file. If for some reason there is a conflict, then preferring "gtest_filter" seems reasonable since it has higher specificity, and is a whitelist rather than a blacklist.
,
Nov 1
If a user is specifying both, I don't see why we'd have reason to think one is preferred over the other. (I do get the recipe case.) I suspect, given how the recipes use the filter flag, that we instead want to teach the scripts how to merge filters and then support merging if multiple --gtest_filters are provided or if --gtest_filter and --test-launcher-filter-file are provided.
,
Nov 1
You have more context than me on the best approach here. I generally prefer the simplest approach that will solve the current problems that we're seeing, but if you think that filter merging is the right approach, then I'll implement that instead.
,
Nov 1
hrm. we don't really want the sum of the two filters per say; more like two independent filters... but implementing that involves basically rewriting the filter system in the android test runner, which seems a bit much. I'm wary of surprising people, but perhaps preferring --gtest_filter in the presence of both is the right course of action. (or perhaps the recipe should fiddle with the command-line when it adds --gtest_filter?)
,
Nov 1
> or perhaps the recipe should fiddle with the command-line when it adds --gtest_filter This is currently tricky to do robustly because test launcher filters are currently added as config strings. We'd need to parse the strings and operate on them, which I'd like to avoid if possible. > I'm wary of surprising people, but perhaps preferring --gtest_filter in the presence of both is the right course of action. We could emit a warning message? That way recipes will work fine, and if a user is trying to run things locally they'll get a warning. |
||
►
Sign in to add a comment |
||
Comment 1 by erikc...@chromium.org
, Nov 1