Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(80)

Issue 26883002: Introduce base::TestSuite::DisableTests(). (Closed)

Created:
7 years, 2 months ago by xhwang
Modified:
7 years, 2 months ago
Reviewers:
Paweł Hajdan Jr.
CC:
chromium-reviews, feature-media-reviews_chromium.org, erikwright+watch_chromium.org
Visibility:
Public.

Description

Introduce base::TestSuite::DisableTests(). Disables tests that match the |filter|. This gives the test suite an opportunity to disable tests at runtime. Must be called before Run() to take effect. For example, to disable "FooTest" when IsFooSupported() is false: if (!IsFooSupported()) DisableTests("FooTest.*"); Run(); BUG=304956 TEST=Tested on Nexus 7 v2.

Patch Set 1 #

Total comments: 1

Patch Set 2 : now works on Android #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -84 lines) Patch
M base/test/test_suite.h View 1 chunk +10 lines, -0 lines 0 comments Download
M base/test/test_suite.cc View 1 1 chunk +12 lines, -0 lines 0 comments Download
M media/base/android/media_codec_bridge_unittest.cc View 3 chunks +0 lines, -12 lines 0 comments Download
M media/base/android/media_source_player_unittest.cc View 16 chunks +2 lines, -72 lines 0 comments Download
M media/base/run_all_unittests.cc View 1 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Paweł Hajdan Jr.
"not lgtm" We can discuss some possibly better ways to handle this. https://codereview.chromium.org/26883002/diff/1/base/test/test_suite.cc File base/test/test_suite.cc ...
7 years, 2 months ago (2013-10-10 18:10:42 UTC) #1
xhwang
7 years, 2 months ago (2013-10-10 18:56:59 UTC) #2
On 2013/10/10 18:10:42, Paweł Hajdan Jr. wrote:
> "not lgtm"
> 
> We can discuss some possibly better ways to handle this.
> 
> https://codereview.chromium.org/26883002/diff/1/base/test/test_suite.cc
> File base/test/test_suite.cc (right):
> 
>
https://codereview.chromium.org/26883002/diff/1/base/test/test_suite.cc#newco...
> base/test/test_suite.cc:147: ::testing::FLAGS_gtest_filter = gtest_filter;
> Please don't do that. By the time you change it it may be too late, e.g. with
> base/test/launcher/test_launcher.

Do you have thoughts on better ways to handle this? Or will it be acceptable if
I can also make test_launcher work with it?

Powered by Google App Engine
This is Rietveld 408576698