wpt manifest fails (and blocks presubmit checks) when local MANIFEST.json is invalid |
|||||
Issue description
Uploads ("git cl upload") are failing with
** Presubmit ERRORS **
lint-test-expectations failed to produce output; check by hand.
When I run lint-test-expectations, it exits with status code 1 and no output. It's dying inside the call to WPTManifest.ensure_manifest which happens before logging is set up.
If I move the ensure_manifest call to run after set_up_logging in run_checks, I can see the stack trace:
Generating MANIFEST.json for web-platform-tests ...
# ret> 1
INFO:manifest:Skipping manifest download because existing file is recent
Traceback (most recent call last):
File "/usr/local/google/home/skobes/c/a/src/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/wpt", line 5, in <module>
wpt.main()
File "/usr/local/google/home/skobes/c/a/src/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/wpt/wpt.py", line 132, in main
rv = script(*args, **kwargs)
File "/usr/local/google/home/skobes/c/a/src/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/manifest/update.py", line 96, in run
update_from_cli(**kwargs)
File "/usr/local/google/home/skobes/c/a/src/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/manifest/update.py", line 41, in update_from_cli
m = manifest.load(tests_root, path)
File "/usr/local/google/home/skobes/c/a/src/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/manifest/manifest.py", line 225, in load
rv = Manifest.from_json(tests_root, json.load(f))
File "/usr/lib/python2.7/json/__init__.py", line 291, in load
**kw)
File "/usr/lib/python2.7/json/__init__.py", line 339, in loads
return _default_decoder.decode(s)
File "/usr/lib/python2.7/json/decoder.py", line 364, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/lib/python2.7/json/decoder.py", line 380, in raw_decode
obj, end = self.scan_once(s, idx)
ValueError: Expecting object: line 95582 column 3 (char 2157720)
,
Mar 14 2018
I used --bypass-hooks to work around the bug. My patch is http://crrev.com/c/959567. However, I don't think it's related to the contents of the patch, because I see the same behavior of lint-test-expectations on origin/master.
,
Mar 14 2018
No wonder I can't reproduce it. `wpt manifest` tries to incrementally update the local manifest. It might not be able to handle corrupt local manifest gracefully, in which case it's an upstream bug. I'll take a look and file an issue upstream. We can also do something in Blink to prevent it from happening. The file is in .gitignore to prevent conflicts, but we do have a version-controlled, valid copy at external/WPT_BASE_MANIFEST.json (used in some other places). We can make sure every time when we need the manifest the base version is copied to external/wpt. Marking the issue as a P3 now since this is the first time I've seen this bug.
,
Apr 4 2018
Saw again on Luke's machine. As manifest update takes a relatively large portion of the presubmit checks, it's actually quite easy to be interrupted and lead to this broken state. Increasing priority and starting to fix it.
,
Apr 4 2018
Note, fergal@ put together a fix inside of wpt/tools/manifest/manifest.py: https://crrev.com/c/994833. How does that look? - Maybe we could submit that and also submit the same fix upstream?
,
Apr 4 2018
,
Apr 4 2018
Thank you, fegral@! That looks like a more proper fix than my WIP attempt (when `wpt manifest` fails, copy WPT_BASE_MANIFEST.json to MANIFEST.json and retry). And it would benefit more people by fixing in the upstream. Yes, sending that PR is the right way to do. The Chromium copy of wpt tools isn't meant to be modified. Once that PR lands in upstream, I'll roll it into Chromium.
,
Apr 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7 commit 31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7 Author: Robert Ma <robertma@chromium.org> Date: Thu Apr 05 22:12:06 2018 Roll in new version of WPT tools This roll is mainly to include https://github.com/w3c/web-platform-tests/pull/10323, which fixes a bug that `wpt manifest` would fail when the local manifest is invalid. wpt.config.json is changed to accomodate: https://github.com/w3c/web-platform-tests/pull/9998 https://github.com/w3c/web-platform-tests/pull/10078 Bug: 822041 Change-Id: I074afd90e32db299fc6f58b870f9b72576fab225 Reviewed-on: https://chromium-review.googlesource.com/997764 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Commit-Queue: Robert Ma <robertma@chromium.org> Cr-Commit-Position: refs/heads/master@{#548584} [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/README.chromium [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/checkout.sh [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt.config.json [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/.gitignore [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/config.default.json [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/gitignore/gitignore.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/lint/lint.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/manifest/manifest.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/serve/serve.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/wpt/browser.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/wpt/commands.json [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/wpt/run.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/wptserve/wptserve/pipes.py [modify] https://crrev.com/31f5e5a24a4e610cda1a1a43f36b0c9916a6a6a7/third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/wptserve/wptserve/server.py
,
Apr 5 2018
Fix has been rolled in. Huge thanks to fergal@!
,
Apr 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df02622c917b3de37971bc2b9ee04c3edfa4ffeb commit df02622c917b3de37971bc2b9ee04c3edfa4ffeb Author: Robert Ma <robertma@chromium.org> Date: Fri Apr 06 01:40:21 2018 [webkitpy] Improve some error handling and logging This CL contains some code health improvements as a by product of fixing issue 822041 . Though the root cause of that issue was eventually fixed from upstream (https://github.com/w3c/web-platform-tests/pull/10323), these improvements are still good to have. * WPTManifest raises an exception when `wpt manifest` fails instead of directy exiting the process. Subprocess invocation is simplified. * lint-test-expectations now has a --verbose flag to print debug logs, including error messages from subprocesses (i.e. `wpt manifest`). * Use the canonical configure_logging in lint-test-expectations and LoggingTestCase in its unit test. Also make the unit test stricter as PRESUBMIT.py relies on the output. Bug: 822041 Change-Id: I4724904c61b3c5b6ed3239584ed03089f2b203ce Reviewed-on: https://chromium-review.googlesource.com/998341 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Commit-Queue: Robert Ma <robertma@chromium.org> Cr-Commit-Position: refs/heads/master@{#548632} [modify] https://crrev.com/df02622c917b3de37971bc2b9ee04c3edfa4ffeb/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive_mock.py [modify] https://crrev.com/df02622c917b3de37971bc2b9ee04c3edfa4ffeb/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/lint_test_expectations.py [modify] https://crrev.com/df02622c917b3de37971bc2b9ee04c3edfa4ffeb/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/lint_test_expectations_unittest.py [modify] https://crrev.com/df02622c917b3de37971bc2b9ee04c3edfa4ffeb/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_manifest.py [modify] https://crrev.com/df02622c917b3de37971bc2b9ee04c3edfa4ffeb/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_manifest_unittest.py
,
Apr 26 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by robertma@chromium.org
, Mar 14 2018