Re-enable reading list tests

ios/chrome/browser/ui/reading_list/reading_list_egtest.mm

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import <EarlGrey/EarlGrey.h>
 #import <UIKit/UIKit.h>
 #import <XCTest/XCTest.h>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ skipping 49 matching lines @@
 const CFTimeInterval kSnackbarAppearanceTimeout = 5;
 const CFTimeInterval kSnackbarDisappearanceTimeout =
     MDCSnackbarMessageDurationMax + 1;
 const CFTimeInterval kDelayForSlowWebServer = 4;
 const CFTimeInterval kLoadOfflineTimeout = kDelayForSlowWebServer + 1;
 const CFTimeInterval kLongPressDuration = 1.0;
 const CFTimeInterval kDistillationTimeout = 5;
 const CFTimeInterval kServerOperationDelay = 1;
 const char kReadHeader[] = "Read";
 const char kUnreadHeader[] = "Unread";
+GURL kDistillableFullURL;
 
 // Overrides the NetworkChangeNotifier to enable distillation even if the device
 // does not have network.
 class WifiNetworkChangeNotifier : public net::NetworkChangeNotifier {
  public:
   WifiNetworkChangeNotifier() : net::NetworkChangeNotifier() {}
 
   ConnectionType GetCurrentConnectionType() const override {
     return CONNECTION_WIFI;
   }
@@ skipping 250 matching lines @@
     };
     GREYAssert(testing::WaitUntilConditionOrTimeout(kLoadOfflineTimeout,
                                                     wait_for_loading),
                @"Page did not load.");
   } else {
     [ChromeEarlGrey waitForStaticHTMLViewContainingText:contentToKeep];
   }
 
   // Test Omnibox URL
   [[EarlGrey selectElementWithMatcher:chrome_test_util::OmniboxText(
-                                          web::test::HttpServer::MakeUrl(
-                                              kDistillableURL)
-                                              .GetContent())]
+                                          kDistillableFullURL.GetContent())]
       assertWithMatcher:grey_notNil()];
 
   // Test presence of online page
   [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewContainingText(
                                           kContentToKeep)]
       assertWithMatcher:online ? grey_notNil() : grey_nil()];
 
   // Test presence of offline page.
   if (online) {
     [ChromeEarlGrey waitForStaticHTMLViewNotContainingText:contentToKeep];
@@ skipping 15 matching lines @@
 
 // Test class for the Reading List menu.
 @interface ReadingListTestCase : ChromeTestCase
 
 @end
 
 @implementation ReadingListTestCase
 
 - (void)setUp {
   [super setUp];
+  kDistillableFullURL = web::test::HttpServer::MakeUrl(kDistillableURL);

[baxley, 2017/06/06 18:14:16]

Can we just replace kDistillableRullURL on line 340 with the right side of this,
and then delete the global?

[huangml1, 2017/06/07 18:10:54]

Yes we can! Sorry that I mixed it up with previous approach when server is
killed the makeUrl does not work at line 340.

   ReadingListModel* model = GetReadingListModel();
   for (const GURL& url : model->Keys())
     model->RemoveEntryByURL(url);
 }
 
 - (void)tearDown {
   web::test::HttpServer& server = web::test::HttpServer::GetSharedInstance();
   if (!server.IsRunning()) {
     server.StartOrDie();
     base::test::ios::SpinRunLoopWithMinDelay(
@@ skipping 55 matching lines @@
       assertWithMatcher:grey_notNil()];
 
   // Verify that the webState's title is correct.
   XCTAssertTrue(chrome_test_util::GetCurrentWebState()->GetTitle() ==
                 base::ASCIIToUTF16(pageTitle.c_str()));
 }
 
 // Tests that sharing a web page to the Reading List results in a snackbar
 // appearing, and that the Reading List entry is present in the Reading List.
 // Loads online version by tapping on entry.
-// TODO(crbug.com/724555): Re-enable the test.
-- (void)DISABLED_testSavingToReadingListAndLoadNormal {
+- (void)testSavingToReadingListAndLoadNormal {
   auto network_change_disabler =
       base::MakeUnique<net::NetworkChangeNotifier::DisableForTest>();
   auto wifi_network = base::MakeUnique<WifiNetworkChangeNotifier>();
   web::test::SetUpSimpleHttpServer(ResponsesForDistillationServer());
   web::test::HttpServer& server = web::test::HttpServer::GetSharedInstance();
   std::string pageTitle(kDistillableTitle);
 
   // Open http://potato
   [ChromeEarlGrey loadURL:web::test::HttpServer::MakeUrl(kDistillableURL)];
 
   AddCurrentPageToReadingList();
 
   // Navigate to http://beans
   [ChromeEarlGrey loadURL:web::test::HttpServer::MakeUrl(kNonDistillableURL)];
   [ChromeEarlGrey waitForPageToFinishLoading];
 
   // Verify that an entry with the correct title is present in the reading list.
   OpenReadingList();
   AssertEntryVisible(pageTitle);
   WaitForDistillation();
 
   // Long press the entry, and open it offline.
   TapEntry(pageTitle);
 
   AssertIsShowingDistillablePage(true);
   // Stop server to reload offline.
-  server.Stop();
+  server.setSuspend(YES);
   base::test::ios::SpinRunLoopWithMinDelay(
       base::TimeDelta::FromSecondsD(kServerOperationDelay));
 
   chrome_test_util::GetCurrentWebState()->GetNavigationManager()->Reload(
       web::ReloadType::NORMAL, false);
   AssertIsShowingDistillablePage(false);
+  server.setSuspend(NO);

[baxley, 2017/06/06 18:14:17]

What if the assert above fails? will the server be unsuspended for the next test
(or teardown)?

[huangml1, 2017/06/07 18:10:54]

You're right.  I missed the condition.  Now I stop suspending the server when
tearing down each test to make the following one in a clean state.

 }
 
 // Tests that sharing a web page to the Reading List results in a snackbar
 // appearing, and that the Reading List entry is present in the Reading List.
 // Loads offline version by tapping on entry without web server.
-// TODO(crbug.com/724555): Re-enable the test.
-- (void)DISABLED_testSavingToReadingListAndLoadNoNetwork {
+- (void)testSavingToReadingListAndLoadNoNetwork {
   auto network_change_disabler =
       base::MakeUnique<net::NetworkChangeNotifier::DisableForTest>();
   auto wifi_network = base::MakeUnique<WifiNetworkChangeNotifier>();
   web::test::SetUpSimpleHttpServer(ResponsesForDistillationServer());
   std::string pageTitle(kDistillableTitle);
   web::test::HttpServer& server = web::test::HttpServer::GetSharedInstance();
   // Open http://potato
   [ChromeEarlGrey loadURL:web::test::HttpServer::MakeUrl(kDistillableURL)];
 
   AddCurrentPageToReadingList();
 
   // Navigate to http://beans
 
   [ChromeEarlGrey loadURL:web::test::HttpServer::MakeUrl(kNonDistillableURL)];
   [ChromeEarlGrey waitForPageToFinishLoading];
 
   // Verify that an entry with the correct title is present in the reading list.
   OpenReadingList();
   AssertEntryVisible(pageTitle);
   WaitForDistillation();
 
   // Stop server to generate error.
-  server.Stop();
+  server.setSuspend(YES);
   base::test::ios::SpinRunLoopWithMinDelay(
       base::TimeDelta::FromSecondsD(kServerOperationDelay));
   // Long press the entry, and open it offline.
   TapEntry(pageTitle);
 
   AssertIsShowingDistillablePage(false);
   // Start server to reload online error.
-  server.StartOrDie();
+  server.setSuspend(NO);

[baxley, 2017/06/06 18:14:16]

Same as last question, will this always get set properly, if something fails
between the two calls to setSuspend?

   base::test::ios::SpinRunLoopWithMinDelay(
       base::TimeDelta::FromSecondsD(kServerOperationDelay));
   web::test::SetUpSimpleHttpServer(ResponsesForDistillationServer());
 
   chrome_test_util::GetCurrentWebState()->GetNavigationManager()->Reload(
       web::ReloadType::NORMAL, false);
   AssertIsShowingDistillablePage(true);
 }
 
 // Tests that sharing a web page to the Reading List results in a snackbar
@@ skipping 262 matching lines @@
       assertWithMatcher:grey_nil()];
   TapButtonWithID(IDS_IOS_READING_LIST_EDIT_BUTTON);
   TapButtonWithID(IDS_IOS_READING_LIST_DELETE_ALL_READ_BUTTON);
 
   // Verify the background string is displayed.
   [[EarlGrey selectElementWithMatcher:EmptyBackground()]
       assertWithMatcher:grey_notNil()];
 }
 
 @end