Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1515833004:
Add missing data_deps to webkit_unit_tests (Closed)
DescriptionAdd missing data_deps to webkit_unit_tests
This causes all webkit_unit_tests failure on CI.
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000
BUG=432959
TBR=dpranke@chromium.org
TBR=brettw@chromium.org
NOTRY=true
Committed: https://crrev.com/5915500329aac3cd4c75fa2ba905ca5954086cba
Cr-Commit-Position: refs/heads/master@{#364373}
Patch Set 1 #
Total comments: 2
Patch Set 2 : +test data dirs #Patch Set 3 : +group #
Total comments: 2
Messages
Total messages: 35 (15 generated)
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1515833004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515833004/1
Description was changed from ========== Add missing data_deps to webkit_unit_tests BUG= ========== to ========== Add missing data_deps to webkit_unit_tests BUG= ==========
tzik@chromium.org changed reviewers: + brettw@chromium.org, dpranke@chromium.org, engedy@chromium.org
Description was changed from ========== Add missing data_deps to webkit_unit_tests BUG= ========== to ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG= ==========
Description was changed from ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG= ========== to ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG= TBR=dpranke@chromium.org TBR=brettw@chromium.org ==========
https://codereview.chromium.org/1515833004/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/BUILD.gn (right): https://codereview.chromium.org/1515833004/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/BUILD.gn:133: "//content/shell:pak", Shouldn't this somehow include the test data that appears in: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
https://codereview.chromium.org/1515833004/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/web/BUILD.gn (right): https://codereview.chromium.org/1515833004/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/web/BUILD.gn:133: "//content/shell:pak", On 2015/12/10 13:54:33, engedy wrote: > Shouldn't this somehow include the test data that appears in: > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... Added. Yes, it's likely needed.
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1515833004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515833004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...) linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1515833004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515833004/40001
It looks working now. engedy: Could you double check to it as a sheriff? dpranke: PTAL later.
Trying it out now by creating an .isolate locally. In the meantime, is there a trybot that uses isolate tests?
On 2015/12/10 15:13:07, engedy wrote: > Trying it out now by creating an .isolate locally. In the meantime, is there a > trybot that uses isolate tests? Yes, I think linux_chromium_rel_ng uses it. That's compile phase doesn't contain ".isolate" and has "isolate_test" phase. http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
On 2015/12/10 15:30:49, tzik wrote: > On 2015/12/10 15:13:07, engedy wrote: > > Trying it out now by creating an .isolate locally. In the meantime, is there > a > > trybot that uses isolate tests? > > Yes, I think linux_chromium_rel_ng uses it. > That's compile phase doesn't contain ".isolate" and has "isolate_test" phase. > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_... Yes, but I don't think it creates the webkit_unit_tests.isolate.
OK, to report back, the .isolated file created by `mb isolate` looks sane. The trybot failure on Win is unrelated, I suggest we land this and see what happens.
On 2015/12/10 15:50:50, engedy wrote: > OK, to report back, the .isolated file created by `mb isolate` looks sane. The > trybot failure on Win is unrelated, I suggest we land this and see what happens. Also `python tools/swarming_client/isolate.py run -s out/ReleaseGN/webkit_unit_tests.isolated` also runs fine.
Description was changed from ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG= TBR=dpranke@chromium.org TBR=brettw@chromium.org ========== to ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG=432959 TBR=dpranke@chromium.org TBR=brettw@chromium.org NOTRY=true ==========
The CQ bit was unchecked by engedy@chromium.org
The CQ bit was checked by engedy@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by engedy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1515833004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515833004/40001
LGTM FWIW.
Message was sent while issue was closed.
Description was changed from ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG=432959 TBR=dpranke@chromium.org TBR=brettw@chromium.org NOTRY=true ========== to ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG=432959 TBR=dpranke@chromium.org TBR=brettw@chromium.org NOTRY=true ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG=432959 TBR=dpranke@chromium.org TBR=brettw@chromium.org NOTRY=true ========== to ========== Add missing data_deps to webkit_unit_tests This causes all webkit_unit_tests failure on CI. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/58000 BUG=432959 TBR=dpranke@chromium.org TBR=brettw@chromium.org NOTRY=true Committed: https://crrev.com/5915500329aac3cd4c75fa2ba905ca5954086cba Cr-Commit-Position: refs/heads/master@{#364373} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/5915500329aac3cd4c75fa2ba905ca5954086cba Cr-Commit-Position: refs/heads/master@{#364373}
Message was sent while issue was closed.
lgtm, thanks! https://codereview.chromium.org/1515833004/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/BUILD.gn (right): https://codereview.chromium.org/1515833004/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/web/BUILD.gn:112: "../core/paint/test_data/", nit: these should be src-relative paths, like //third_party/WebKit/Source/core/paint/test_data . https://codereview.chromium.org/1515833004/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/web/BUILD.gn:148: ":webkit_unit_tests_data", and the presubmit check should've complained that this was sorted wrong; maybe there's a bug there where we're not sorting data_deps ...
Message was sent while issue was closed.
On 2015/12/10 17:28:37, Dirk Pranke wrote: > lgtm, thanks! > > https://codereview.chromium.org/1515833004/diff/40001/third_party/WebKit/Sour... > File third_party/WebKit/Source/web/BUILD.gn (right): > > https://codereview.chromium.org/1515833004/diff/40001/third_party/WebKit/Sour... > third_party/WebKit/Source/web/BUILD.gn:112: "../core/paint/test_data/", > nit: these should be src-relative paths, like > //third_party/WebKit/Source/core/paint/test_data . > > https://codereview.chromium.org/1515833004/diff/40001/third_party/WebKit/Sour... > third_party/WebKit/Source/web/BUILD.gn:148: ":webkit_unit_tests_data", > and the presubmit check should've complained that this was sorted wrong; maybe > there's a bug there where we're not sorting data_deps ... Thanks for reviewing! Yeah, it's just sloppy sheriff work... :-) Could you please address those in a follow-up CL?
Message was sent while issue was closed.
On 2015/12/10 17:32:17, engedy wrote: > Thanks for reviewing! Yeah, it's just sloppy sheriff work... :-) Could you > please address those in a follow-up CL? Yup, will do. |
