New issue
Advanced search Search tips

Issue 656171 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 693792

Blocking:
issue 621599



Sign in to add a comment

[W3C import] Patch failure occurs in try jobs when some non-ascii characters appear in imported files

Project Member Reported by qyears...@chromium.org, Oct 14 2016

Issue description

The latest runs of w3c-test-autoroller are now having try jobs which fail to apply the patch.

Example run:
https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller/builds/7751

Corresponding CL:
https://codereview.chromium.org/2420733003

Example try job from that CL:
https://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/builds/1199

Patch error message excerpt:
  error: patch failed: third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js:1
  error: third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js: patch does not apply

Patch:       third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js
Index: third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js
diff --git a/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js b/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js
index 323d1c936c0fd6c46529caa292505970925a6c38..28489a572be1ccb7f500daf04184e6cf73e8353f 100644
--- a/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js
+++ b/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js
@@ -1,5 +1,5 @@
 importScripts("/resources/testharness.js");
 test(function() {
-  assert_equals("ÿ", "\ufffd");
+  assert_equals("ÿ", "\ufffd");
 }, "Decoding invalid utf-8");
 done();
 

Comment 1 by falken@chromium.org, Oct 18 2016

Cc: nhiroki@chromium.org
Labels: -Pri-2 Pri-1
Would anyone be able to get to this soon? If not, it seems like skipping 004.worker.js is a reasonable workaround so that W3C tests can continue to be imported.
I was hoping to look at this tomorrow; as of right now, I'm still not sure where the problem is.

Meanwhile, I think that skipping 004.worker.js until this is fixed is reasonable in any case... uploaded http://crrev.com/2429523003.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/096c5be32530dd75f9912c0d08230149195cf61d

commit 096c5be32530dd75f9912c0d08230149195cf61d
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Oct 19 02:17:59 2016

Skip wpt/workers/semantics/encodings until  crbug.com/656171  is resolved.

BUG= 656171 

Review-Url: https://chromiumcodereview.appspot.com/2429523003
Cr-Commit-Position: refs/heads/master@{#426118}

[modify] https://crrev.com/096c5be32530dd75f9912c0d08230149195cf61d/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/096c5be32530dd75f9912c0d08230149195cf61d/third_party/WebKit/LayoutTests/W3CImportExpectations
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/001.html
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/001.html.headers
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/002.html
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/002.html.headers
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/003-1.py
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/003.html
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.html
[delete] https://crrev.com/4c4977aa052dad072bcf9539f2b4507b084e8f27/third_party/WebKit/LayoutTests/imported/wpt/workers/semantics/encodings/004.worker.js

Labels: -Pri-1 Pri-2
Summary: [W3C auto-import] Patch failure occured when non-ascii characters appeared in test files. (was: w3c-test-autoroller: patch failure occurs when some non-ascii characters appear in tests.)
Components: Blink>Infra>Predictability
Components: -Blink>Infra
Cc: jeffcarp@chromium.org
Summary: [W3C import] Patch failure occurs in try jobs when some non-ascii characters appear in imported files (was: [W3C auto-import] Patch failure occured when non-ascii characters appeared in test files.)
Observed something similar again today:
https://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/builds/4363

Failed to apply patch for third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js:

...

  error: patch failed: third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js:1
  error: third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js: patch does not apply

Patch:       third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js
Index: third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js
diff --git a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js
index 4ac83bf9f87edc505ba589e40ac4e15aea871e11..50de6932ba25bf02d8142688d9fec8fc30d83360 100644
--- a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js
+++ b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js
@@ -1,5 +1,5 @@
 (function() {
   window.getSomeString = function() {
-    return "œæ¹¿Ÿ"; //<- these are five Polish letters, similar to scazz. It can be read correctly only with windows 1250 encoding.
+    return "œæ¹¿Ÿ"; //<- these are five Polish letters, similar to scazz. It can be read correctly only with windows 1250 encoding.
   };
 })();
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7082389df8da5959441981c484ffd94f3190928

commit c7082389df8da5959441981c484ffd94f3190928
Author: qyearsley <qyearsley@chromium.org>
Date: Tue Jan 31 19:43:06 2017

Skip some tests that depend on a file with a non-utf8 encoding.

TBR=jeffcarp,tkent
TBR_REASON=Unblocking the W3C test autoroller
BUG= 656171 

Review-Url: https://codereview.chromium.org/2666113002
Cr-Commit-Position: refs/heads/master@{#447303}

[modify] https://crrev.com/c7082389df8da5959441981c484ffd94f3190928/third_party/WebKit/LayoutTests/W3CImportExpectations
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/external-script-windows1250.js
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/script-charset-01-expected.txt
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/script-charset-01.html
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/script-charset-02-expected.txt
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/script-charset-02.html
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/script-charset-03-expected.txt
[delete] https://crrev.com/4aa8cbb78f4dbd9437a460b33b6bb54f1fdc275c/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/script-charset-03.html

Closely related bug:  bug 691107 .

Update: the root cause here appears to be that apply_issue.py (https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/apply_issue.py) is used to apply patches from Rietveld, which uses a module called fix_encoding, which forces utf-8.

This could either be changed, or we could switch to Gerrit for all imports which would likely fix this issue.
Blockedon: 693792
Note, I expect this to be resolved after migrating to Gerrit ( bug 693792 ).
Owner: qyears...@chromium.org
Status: Assigned (was: Available)
Still left to do here: Re-enable any tests that were disabled due to this bug, and verify that import works OK with Gerrit.
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5b382e1147bcc0f6a6ba5e31ddb0c107564d5a3

commit b5b382e1147bcc0f6a6ba5e31ddb0c107564d5a3
Author: qyearsley <qyearsley@chromium.org>
Date: Thu Mar 02 21:13:34 2017

W3C test importer: Unskip some files with non-utf8 encodings.

In theory since imports should now be done by Gerrit, this issue may be resolved.

BUG= 656171 

Review-Url: https://codereview.chromium.org/2722723003
Cr-Commit-Position: refs/heads/master@{#454375}

[modify] https://crrev.com/b5b382e1147bcc0f6a6ba5e31ddb0c107564d5a3/third_party/WebKit/LayoutTests/W3CImportExpectations

Status: Fixed (was: Assigned)
Fixed by migration to Gerrit
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability

Sign in to add a comment