|
|
DescriptionMake linux_layout_tests_slimming_paint_v2 run with dcheck_always_on
This patch updates the slimming paint v2 trybot to run with
dcheck_always_on.
BUG=702805
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Review-Url: https://codereview.chromium.org/2816613002
Cr-Commit-Position: refs/heads/master@{#464071}
Committed: https://chromium.googlesource.com/chromium/src/+/d850820a384d5cad1cbbcdfc209becdaf6beae88
Patch Set 1 #Patch Set 2 : Update expectations #
Messages
Total messages: 23 (14 generated)
pdr@chromium.org changed reviewers: + dpranke@chromium.org, phajdan.jr@chromium.org
Description was changed from ========== Make linux_layout_tests_slimming_paint_v2 run with dcheck_always_on This patch updates the slimming paint v2 trybot to run with dcheck_always_on. BUG=702805 ========== to ========== Make linux_layout_tests_slimming_paint_v2 run with dcheck_always_on This patch updates the slimming paint v2 trybot to run with dcheck_always_on. BUG=702805 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
The CQ bit was checked by pdr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Dirk and Paweł, This is my first time touching this sort of config file. I think this change is correct but please review this like I have no idea what I'm doing.
You realize that if you enable DCHECKs the layout test expectations don't really work right, right?
On 2017/04/11 at 18:56:33, dpranke wrote: > You realize that if you enable DCHECKs the layout test expectations don't really work right, right? I didn't realize that. Why is that the case? I prepared for enabling dchecks by running the layout tests with dchecks on and adding ~200 lines that are crashing with dchecks. I was expecting the expectations to work though.
On 2017/04/11 20:37:59, pdr. wrote: > On 2017/04/11 at 18:56:33, dpranke wrote: > > You realize that if you enable DCHECKs the layout test expectations don't > really work right, right? > > I didn't realize that. Why is that the case? > > I prepared for enabling dchecks by running the layout tests with dchecks on and > adding ~200 lines that are crashing with dchecks. I was expecting the > expectations to work though. The "Release" and "Debug" configurations in TestExpectations match what the waterfalls are (release build + no dcheck, full debug including dcheck). When you run a build with release+dcheck, the dchecks trigger, and you get many of the failures you'd expect from a Debug build, but you're in the wrong configuration. So, your results don't necessarily match release, and they don't necessarily match full debug. And, we don't support a third configuration. This doesn't happen all the time, but it does happen some of the time. When we run the layout tests on the CQ builders, dchecks are also enabled, so I think most of the failures are already accounted for, but I wanted to make sure you understood what would happen. LGTM if you want to land this.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)
The CQ bit was checked by pdr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
On 2017/04/11 at 21:19:33, dpranke wrote: > On 2017/04/11 20:37:59, pdr. wrote: > > On 2017/04/11 at 18:56:33, dpranke wrote: > > > You realize that if you enable DCHECKs the layout test expectations don't > > really work right, right? > > > > I didn't realize that. Why is that the case? > > > > I prepared for enabling dchecks by running the layout tests with dchecks on and > > adding ~200 lines that are crashing with dchecks. I was expecting the > > expectations to work though. > > The "Release" and "Debug" configurations in TestExpectations match what the waterfalls > are (release build + no dcheck, full debug including dcheck). > > When you run a build with release+dcheck, the dchecks trigger, and you get many of the failures you'd expect from a Debug build, but you're in the wrong configuration. So, your results don't necessarily match release, and they don't necessarily match full debug. And, we don't support a third configuration. > > This doesn't happen all the time, but it does happen some of the time. > > When we run the layout tests on the CQ builders, dchecks are also enabled, so I think most of the failures are already accounted for, but I wanted to make sure you understood what would happen. > > LGTM if you want to land this. Thank you for the info! I did not know about this, but I'm familiar with the release+dcheck weirdness. I will keep this in mind.
The CQ bit was checked by pdr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/2816613002/#ps20001 (title: "Update expectations")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492018239080760, "parent_rev": "91b3f7e7b1f592052614ffd6eb61d60f87dd2571", "commit_rev": "d850820a384d5cad1cbbcdfc209becdaf6beae88"}
Message was sent while issue was closed.
Description was changed from ========== Make linux_layout_tests_slimming_paint_v2 run with dcheck_always_on This patch updates the slimming paint v2 trybot to run with dcheck_always_on. BUG=702805 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Make linux_layout_tests_slimming_paint_v2 run with dcheck_always_on This patch updates the slimming paint v2 trybot to run with dcheck_always_on. BUG=702805 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2816613002 Cr-Commit-Position: refs/heads/master@{#464071} Committed: https://chromium.googlesource.com/chromium/src/+/d850820a384d5cad1cbbcdfc209b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/d850820a384d5cad1cbbcdfc209b... |