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

Issue 857863002: Revert of Introduce new test for bookmark loading. (Closed)

Created:
5 years, 11 months ago by Alexandre Carlton
Modified:
5 years, 10 months ago
Reviewers:
Sam McNally, raymes
Base URL:
https://chromium.googlesource.com/chromium/src.git@bookmarks-minimal
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Introduce new test for bookmark loading. (patchset #8 id:130001 of https://codereview.chromium.org/840493002/) Reason for revert: Failing on bots http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%29/builds/5305/steps/browser_tests/logs/Bookmark Original issue's description: > Introduce new test for bookmark loading. > > BUG=110020 > > Committed: https://crrev.com/50567ebbd5ca2ff79d87382addc11d25658776d2 > Cr-Commit-Position: refs/heads/master@{#312076} TBR=raymes@chromium.org,sammc@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=110020

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -61 lines) Patch
M chrome/browser/resources/pdf/pdf.js View 1 chunk +0 lines, -11 lines 0 comments Download
M chrome/browser/resources/pdf/pdf_extension_test.cc View 3 chunks +6 lines, -11 lines 0 comments Download
D chrome/test/data/pdf/bookmarks_test.js View 1 chunk +0 lines, -39 lines 0 comments Download
D chrome/test/data/pdf/test-bookmarks.pdf View Binary file 0 comments Download

Messages

Total messages: 13 (5 generated)
Alexandre Carlton
Created Revert of Introduce new test for bookmark loading.
5 years, 11 months ago (2015-01-19 22:54:36 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/857863002/1
5 years, 11 months ago (2015-01-19 22:55:12 UTC) #2
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 11 months ago (2015-01-19 22:55:14 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/857863002/1
5 years, 11 months ago (2015-01-19 23:28:42 UTC) #6
Sam McNally
lgtm
5 years, 11 months ago (2015-01-19 23:28:43 UTC) #7
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 11 months ago (2015-01-19 23:28:44 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/857863002/1
5 years, 11 months ago (2015-01-19 23:28:53 UTC) #11
commit-bot: I haz the power
5 years, 11 months ago (2015-01-19 23:29:11 UTC) #13
Failed to apply patch for chrome/browser/resources/pdf/pdf_extension_test.cc:
While running git apply --index -3 -p1;
  error: patch failed: chrome/browser/resources/pdf/pdf_extension_test.cc:87
  Falling back to three-way merge...
  Applied patch to 'chrome/browser/resources/pdf/pdf_extension_test.cc' with
conflicts.
  U chrome/browser/resources/pdf/pdf_extension_test.cc

Patch:       chrome/browser/resources/pdf/pdf_extension_test.cc
Index: chrome/browser/resources/pdf/pdf_extension_test.cc
diff --git a/chrome/browser/resources/pdf/pdf_extension_test.cc
b/chrome/browser/resources/pdf/pdf_extension_test.cc
index
2ba39addc1ea6106f4dba3b420a21fa8813ff46f..e738d4fc0cc68ab43d6120be3f98ae4817a8db7d
100644
--- a/chrome/browser/resources/pdf/pdf_extension_test.cc
+++ b/chrome/browser/resources/pdf/pdf_extension_test.cc
@@ -38,12 +38,11 @@
     ExtensionApiTest::TearDownOnMainThread();
   }
 
-  void RunTestsInFile(std::string filename,
-                      std::string pdf_filename) {
+  void RunTestsInFile(std::string filename, bool requiresPlugin) {
     base::FilePath pdf_plugin_src;
     PathService::Get(base::DIR_SOURCE_ROOT, &pdf_plugin_src);
     pdf_plugin_src = pdf_plugin_src.AppendASCII("pdf");
-    if (!base::DirectoryExists(pdf_plugin_src)) {
+    if (requiresPlugin && !base::DirectoryExists(pdf_plugin_src)) {
       LOG(WARNING) << "Not running " << filename <<
           " because it requires the PDF plugin which is not available.";
       return;
@@ -62,7 +61,7 @@
 
     extensions::ResultCatcher catcher;
 
-    GURL url(embedded_test_server()->GetURL("/pdf/" + pdf_filename));
+    GURL url(embedded_test_server()->GetURL("/pdf/test.pdf"));
     GURL extension_url(
         "chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html?" +
         url.spec());
@@ -87,17 +86,13 @@
 };
 
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Basic) {
-  RunTestsInFile("basic_test.js", "test.pdf");
+  RunTestsInFile("basic_test.js", false);
 }
 
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, BasicPlugin) {
-  RunTestsInFile("basic_plugin_test.js", "test.pdf");
+  RunTestsInFile("basic_plugin_test.js", true);
 }
 
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Viewport) {
-  RunTestsInFile("viewport_test.js", "test.pdf");
+  RunTestsInFile("viewport_test.js", false);
 }
-
-IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Bookmark) {
-  RunTestsInFile("bookmarks_test.js", "test-bookmarks.pdf");
-}

Powered by Google App Engine
This is Rietveld 408576698