Commit a7b569e6 authored by Nico Weber's avatar Nico Weber

Enable -Wunreachable-code in iOS builds (except for internal builds).

This requires replacing EARL_GREY_TEST_DISABLED with DISABLED_ prefixes
in cases where the call to EARL_GREY_TEST_DISABLED is unconditional.
earl_grey/disabled_test_macros.h, which defines EARL_GREY_TEST_DISABLED,
says:

    This macro should be used when the configuration for which the
    test should be disabled can only be determined at runtime.
    Disabling at compile-time is always preferred.

So this matches the official guidance, and there are many
egtests that use DISABLED_ already.

Bug: 346399
Change-Id: Ibe51463ec0e6afbd1b3bef629406163118efa64c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125878
Auto-Submit: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754468}
parent 41d660f0
...@@ -1591,7 +1591,8 @@ config("chromium_code") { ...@@ -1591,7 +1591,8 @@ config("chromium_code") {
# TODO(thakis): Enable this more often, https://crbug.com/346399 # TODO(thakis): Enable this more often, https://crbug.com/346399
# use_libfuzzer: https://crbug.com/1063180 # use_libfuzzer: https://crbug.com/1063180
if (!is_nacl && !use_libfuzzer && target_os != "ios") { if (!is_nacl && !use_libfuzzer &&
!(is_ios && ios_app_bundle_id_prefix != "org.chromium")) {
cflags += [ "-Wunreachable-code" ] cflags += [ "-Wunreachable-code" ]
} }
......
...@@ -304,9 +304,7 @@ void V4GetHashProtocolManager::GetFullHashes( ...@@ -304,9 +304,7 @@ void V4GetHashProtocolManager::GetFullHashes(
// TODO(crbug.com/1028755): Enable full hash checks on iOS. // TODO(crbug.com/1028755): Enable full hash checks on iOS.
#if defined(OS_IOS) #if defined(OS_IOS)
std::move(callback).Run(cached_full_hash_infos); std::move(callback).Run(cached_full_hash_infos);
return; #else
#endif
net::NetworkTrafficAnnotationTag traffic_annotation = net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("safe_browsing_v4_get_hash", R"( net::DefineNetworkTrafficAnnotation("safe_browsing_v4_get_hash", R"(
semantics { semantics {
...@@ -361,6 +359,7 @@ void V4GetHashProtocolManager::GetFullHashes( ...@@ -361,6 +359,7 @@ void V4GetHashProtocolManager::GetFullHashes(
clock_->Now())); clock_->Now()));
UMA_HISTOGRAM_COUNTS_100("SafeBrowsing.V4GetHash.CountOfPrefixes", UMA_HISTOGRAM_COUNTS_100("SafeBrowsing.V4GetHash.CountOfPrefixes",
prefixes_to_request.size()); prefixes_to_request.size());
#endif
} }
void V4GetHashProtocolManager::GetFullHashesWithApis( void V4GetHashProtocolManager::GetFullHashesWithApis(
......
...@@ -200,12 +200,13 @@ void SignOut() { ...@@ -200,12 +200,13 @@ void SignOut() {
// //chrome/browser/metrics/ukm_browsertest.cc // //chrome/browser/metrics/ukm_browsertest.cc
// Make sure that UKM is disabled while an incognito tab is open. // Make sure that UKM is disabled while an incognito tab is open.
- (void)testRegularPlusIncognito {
#if defined(CHROME_EARL_GREY_1) #if defined(CHROME_EARL_GREY_1)
// TODO(crbug.com/1033726): EG1 Test fails on iOS 12. // TODO(crbug.com/1033726): EG1 Test fails on iOS 12.
EARL_GREY_TEST_DISABLED(@"EG1 Fails"); #define MAYBE_testRegularPlusIncognito DISABLED_testRegularPlusIncognito
#else
#define MAYBE_testRegularPlusIncognito testRegularPlusIncognito
#endif #endif
- (void)MAYBE_testRegularPlusIncognito {
uint64_t originalClientID = [MetricsAppInterface UKMClientID]; uint64_t originalClientID = [MetricsAppInterface UKMClientID];
OpenNewIncognitoTab(); OpenNewIncognitoTab();
...@@ -235,12 +236,13 @@ void SignOut() { ...@@ -235,12 +236,13 @@ void SignOut() {
} }
// Make sure opening a real tab after Incognito doesn't enable UKM. // Make sure opening a real tab after Incognito doesn't enable UKM.
- (void)testIncognitoPlusRegular {
#if defined(CHROME_EARL_GREY_1) #if defined(CHROME_EARL_GREY_1)
// TODO(crbug.com/1033726): EG1 Test fails on iOS 12. // TODO(crbug.com/1033726): EG1 Test fails on iOS 12.
EARL_GREY_TEST_DISABLED(@"EG1 Fails"); #define MAYBE_testIncognitoPlusRegular DISABLED_testIncognitoPlusRegular
#else
#define MAYBE_testIncognitoPlusRegular testIncognitoPlusRegular
#endif #endif
- (void)MAYBE_testIncognitoPlusRegular {
uint64_t originalClientID = [MetricsAppInterface UKMClientID]; uint64_t originalClientID = [MetricsAppInterface UKMClientID];
[ChromeEarlGrey closeAllTabs]; [ChromeEarlGrey closeAllTabs];
[ChromeEarlGrey waitForMainTabCount:0]; [ChromeEarlGrey waitForMainTabCount:0];
......
...@@ -1207,10 +1207,8 @@ void TestResponseProvider::GetLanguageResponse( ...@@ -1207,10 +1207,8 @@ void TestResponseProvider::GetLanguageResponse(
// Tests that "Never Translate ..." is automatically triggered only for a // Tests that "Never Translate ..." is automatically triggered only for a
// maximum number of times if refused by the user. // maximum number of times if refused by the user.
- (void)testInfobarAutoNeverTranslateMaxTries { // TODO(crbug.com/945118): Re-enable when fixed.
// TODO(crbug.com/945118): Re-enable when fixed. - (void)DISABLED_testInfobarAutoNeverTranslateMaxTries {
EARL_GREY_TEST_DISABLED(@"Test disabled.");
// Start the HTTP server. // Start the HTTP server.
std::unique_ptr<web::DataResponseProvider> provider(new TestResponseProvider); std::unique_ptr<web::DataResponseProvider> provider(new TestResponseProvider);
web::test::SetUpHttpServer(std::move(provider)); web::test::SetUpHttpServer(std::move(provider));
......
...@@ -240,17 +240,18 @@ using chrome_test_util::TappableBookmarkNodeWithLabel; ...@@ -240,17 +240,18 @@ using chrome_test_util::TappableBookmarkNodeWithLabel;
} }
// Test that swiping left to right navigate back. // Test that swiping left to right navigate back.
- (void)testNavigateBackWithGesture {
// Disabled on iPad as there is not "navigate back" gesture.
if ([ChromeEarlGrey isIPadIdiom]) {
EARL_GREY_TEST_SKIPPED(@"Test not applicable for iPad");
}
// TODO(crbug.com/768339): This test is faling on devices because // TODO(crbug.com/768339): This test is faling on devices because
// grey_swipeFastInDirectionWithStartPoint does not work. // grey_swipeFastInDirectionWithStartPoint does not work.
#if !TARGET_IPHONE_SIMULATOR #if !TARGET_IPHONE_SIMULATOR
EARL_GREY_TEST_DISABLED(@"Test disabled on devices."); #define MAYBE_testNavigateBackWithGesture DISABLED_testNavigateBackWithGesture
#else
#define MAYBE_testNavigateBackWithGesture testNavigateBackWithGesture
#endif #endif
- (void)MAYBE_testNavigateBackWithGesture {
// Disabled on iPad as there is not "navigate back" gesture.
if ([ChromeEarlGrey isIPadIdiom]) {
EARL_GREY_TEST_SKIPPED(@"Test not applicable for iPad");
}
if (@available(iOS 13, *)) { if (@available(iOS 13, *)) {
// Navigate back side swipe gesture does not work on iOS13 simulator. This // Navigate back side swipe gesture does not work on iOS13 simulator. This
......
...@@ -532,11 +532,9 @@ void TapSuppressDialogsButton() { ...@@ -532,11 +532,9 @@ void TapSuppressDialogsButton() {
} }
// Tests that an alert is presented after displaying the share menu. // Tests that an alert is presented after displaying the share menu.
- (void)testShowJavaScriptAfterShareMenu { // TODO(crbug.com/747622): re-enable this test once earl grey can interact
// TODO(crbug.com/747622): re-enable this test once earl grey can interact // with the share menu.
// with the share menu. - (void)DISABLED_testShowJavaScriptAfterShareMenu {
EARL_GREY_TEST_DISABLED(@"Disabled until EG can use share menu.");
// Load the blank test page. // Load the blank test page.
const GURL kURL = self.testServer->GetURL(kAlertURLPath); const GURL kURL = self.testServer->GetURL(kAlertURLPath);
[ChromeEarlGrey loadURL:kURL]; [ChromeEarlGrey loadURL:kURL];
......
...@@ -116,11 +116,14 @@ void AssertURLIs(const GURL& expectedURL) { ...@@ -116,11 +116,14 @@ void AssertURLIs(const GURL& expectedURL) {
// Verifies that the toolbar is not hidden when scrolling a short pdf, as the // Verifies that the toolbar is not hidden when scrolling a short pdf, as the
// entire document is visible without hiding the toolbar. // entire document is visible without hiding the toolbar.
- (void)testSmallWidePDFScroll {
// TODO(crbug.com/1022029): Enable this test.
#if defined(CHROME_EARL_GREY_2) #if defined(CHROME_EARL_GREY_2)
EARL_GREY_TEST_DISABLED(@"Fails with EG2"); // TODO(crbug.com/1022029): Enable this test.
#elif defined(CHROME_EARL_GREY_1) #define MAYBE_testSmallWidePDFScroll DISABLED_testSmallWidePDFScroll
#else
#define MAYBE_testSmallWidePDFScroll testSmallWidePDFScroll
#endif
- (void)MAYBE_testSmallWidePDFScroll {
#if defined(CHROME_EARL_GREY_1)
// TODO(crbug.com/1036221): EG1 Test fails on iOS 12. // TODO(crbug.com/1036221): EG1 Test fails on iOS 12.
if (!base::ios::IsRunningOnIOS13OrLater()) { if (!base::ios::IsRunningOnIOS13OrLater()) {
EARL_GREY_TEST_DISABLED(@"EG1 Fails on iOS 12."); EARL_GREY_TEST_DISABLED(@"EG1 Fails on iOS 12.");
......
...@@ -150,10 +150,13 @@ id<GREYMatcher> SearchCopiedTextButton() { ...@@ -150,10 +150,13 @@ id<GREYMatcher> SearchCopiedTextButton() {
// Tests that the XClientData header is sent when navigating to // Tests that the XClientData header is sent when navigating to
// https://google.com through the omnibox. // https://google.com through the omnibox.
- (void)testXClientData {
#if defined(CHROME_EARL_GREY_1) #if defined(CHROME_EARL_GREY_1)
EARL_GREY_TEST_DISABLED(@"Flaky on EG1."); // Flaky on EG1.
#define MAYBE_testXClientData DISABLED_testXClientData
#else
#define MAYBE_testXClientData testXClientData
#endif #endif
- (void)MAYBE_testXClientData {
// Rewrite the google URL to localhost URL. // Rewrite the google URL to localhost URL.
[OmniboxAppInterface rewriteGoogleURLToLocalhost]; [OmniboxAppInterface rewriteGoogleURLToLocalhost];
......
...@@ -133,16 +133,14 @@ using chrome_test_util::SystemSelectionCalloutCopyButton; ...@@ -133,16 +133,14 @@ using chrome_test_util::SystemSelectionCalloutCopyButton;
// Tests whether input mode in an omnibox can be canceled via tapping the typing // Tests whether input mode in an omnibox can be canceled via tapping the typing
// shield and asserts it doesn't commit the omnibox contents if the input is // shield and asserts it doesn't commit the omnibox contents if the input is
// canceled. // canceled.
- (void)testToolbarOmniboxTypingShield { // TODO(crbug.com/753098): Re-enable this test on iPad once grey_typeText
// works on iOS 11.
- (void)DISABLED_testToolbarOmniboxTypingShield {
// Tablet only (handset keyboard does not have "hide keyboard" button). // Tablet only (handset keyboard does not have "hide keyboard" button).
if (![ChromeEarlGrey isIPadIdiom]) { if (![ChromeEarlGrey isIPadIdiom]) {
EARL_GREY_TEST_SKIPPED(@"Test not support on iPhone"); EARL_GREY_TEST_SKIPPED(@"Test not support on iPhone");
} }
// TODO(crbug.com/753098): Re-enable this test on iPad once grey_typeText
// works on iOS 11.
EARL_GREY_TEST_DISABLED(@"Test disabled on iPad.");
const GURL URL = web::test::HttpServer::MakeUrl("http://origin"); const GURL URL = web::test::HttpServer::MakeUrl("http://origin");
[ChromeEarlGrey loadURL:URL]; [ChromeEarlGrey loadURL:URL];
...@@ -203,11 +201,9 @@ using chrome_test_util::SystemSelectionCalloutCopyButton; ...@@ -203,11 +201,9 @@ using chrome_test_util::SystemSelectionCalloutCopyButton;
} }
// Verifies that copying and pasting a URL includes the hidden protocol prefix. // Verifies that copying and pasting a URL includes the hidden protocol prefix.
- (void)testCopyPasteURL { // TODO(crbug.com/834345): Enable this test when long press on the steady
// TODO(crbug.com/834345): Enable this test when long press on the steady // location bar is supported.
// location bar is supported. - (void)DISABLED_testCopyPasteURL {
EARL_GREY_TEST_SKIPPED(@"Test not supported yet in UI Refresh.");
// Clear generalPasteboard before and after the test. // Clear generalPasteboard before and after the test.
[UIPasteboard generalPasteboard].string = @""; [UIPasteboard generalPasteboard].string = @"";
[self setTearDownHandler:^{ [self setTearDownHandler:^{
......
...@@ -18,13 +18,11 @@ ...@@ -18,13 +18,11 @@
namespace ios_web_view { namespace ios_web_view {
void InitializeGlobalState() { void InitializeGlobalState() {
#if defined(UNIT_TEST)
// Do not perform global state initialization in an unit test environment. // Do not perform global state initialization in an unit test environment.
// 1. Not needed when unit testing. // 1. Not needed when unit testing.
// 2. The globals below will try to create other already created globals like // 2. The globals below will try to create other already created globals like
// AtExitManagers. This causes DCHECKs and prevents tests from completing. // AtExitManagers. This causes DCHECKs and prevents tests from completing.
return; #if !defined(UNIT_TEST)
#endif // defined(UNIT_TEST)
static std::unique_ptr<ios_web_view::WebViewWebClient> web_client; static std::unique_ptr<ios_web_view::WebViewWebClient> web_client;
static std::unique_ptr<ios_web_view::WebViewWebMainDelegate> static std::unique_ptr<ios_web_view::WebViewWebMainDelegate>
web_main_delegate; web_main_delegate;
...@@ -51,6 +49,7 @@ void InitializeGlobalState() { ...@@ -51,6 +49,7 @@ void InitializeGlobalState() {
web_client.reset(); web_client.reset();
}]; }];
}); });
#endif // defined(UNIT_TEST)
} }
} // namespace ios_web_view } // namespace ios_web_view
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment