Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2776913002:
[LayoutTests] No longer skip importing payment-request tests from WPT (Closed)
Description[LayoutTests] No longer skip importing payment-request tests from WPT
Tests will be imported at the next auto-import
From running locally, there are 6 that fail (to be investigated). I will submit TestExpectations once the tests are imported
BUG=705252
TEST=None
R=foolip@chromium.org
Review-Url: https://codereview.chromium.org/2776913002 .
Cr-Commit-Position: refs/heads/master@{#460378}
Committed: https://chromium.googlesource.com/chromium/src/+/57a65c934af9dce077f720e6fc8a7579de79924f
Patch Set 1 #Patch Set 2 : Updated test expectations #Patch Set 3 : test expectations for later #Messages
Total messages: 20 (10 generated)
The CQ bit was checked by mathp@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...
Description was changed from ========== [LayoutTests] No longer skip importing payment-request tests from WPT Tests will be imported at the next auto-import BUG=705252 TEST=None ========== to ========== [LayoutTests] No longer skip importing payment-request tests from WPT Tests will be imported at the next auto-import From running locally, there are 6 that fail (to be investigated). I updated the TestExpectations. BUG=705252 TEST=None ==========
mathp@chromium.org changed reviewers: + foolip@chromium.org
Hi Philip, 6/13 of the WPT tests don't pass. I added TestExpectations and will investigate this, but I think this is already a good first step. Process question: Lint complains that the TestExpectations are for non-existent paths, but they will be present once the test import runs. Is this OK or should I wait for the import to happen, then check-in the expectations once they fail? BTW we fully intend to be compliant, currently the spec is in flux and either the tests or our implementation could be at fault. I will investigate. Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
On 2017/03/26 01:33:14, Mathieu wrote: > Hi Philip, > > 6/13 of the WPT tests don't pass. I added TestExpectations and will investigate > this, but I think this is already a good first step. > > Process question: Lint complains that the TestExpectations are for non-existent > paths, but they will be present once the test import runs. Is this OK or should > I wait for the import to happen, then check-in the expectations once they fail? > > BTW we fully intend to be compliant, currently the spec is in flux and either > the tests or our implementation could be at fault. I will investigate. > > Thanks! I see the the chromium_presubmit _really_ doesn't like the non-existent paths. I could (1) land without running presubmits (2) As described earlier, wait for the import and land test expectations Let me know, cheers
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... has some advice for how to do this, and it's to just change W3CImportExpectations and then wait for an import. So this LGTM with the TestExpectations changes reverted. Once the import is done, you can see what failing tests were imported.
The CQ bit was checked by mathp@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org Link to the patchset: https://codereview.chromium.org/2776913002/#ps40001 (title: "test expectations for later")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/03/29 09:10:07, foolip_UTC9 wrote: > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > has some advice for how to do this, and it's to just change > W3CImportExpectations and then wait for an import. So this LGTM with the > TestExpectations changes reverted. Once the import is done, you can see what > failing tests were imported. Sounds good! I will check the import. Does it happen often?
Description was changed from ========== [LayoutTests] No longer skip importing payment-request tests from WPT Tests will be imported at the next auto-import From running locally, there are 6 that fail (to be investigated). I updated the TestExpectations. BUG=705252 TEST=None ========== to ========== [LayoutTests] No longer skip importing payment-request tests from WPT Tests will be imported at the next auto-import From running locally, there are 6 that fail (to be investigated). I will submit TestExpectations once the tests are imported BUG=705252 TEST=None ==========
Description was changed from ========== [LayoutTests] No longer skip importing payment-request tests from WPT Tests will be imported at the next auto-import From running locally, there are 6 that fail (to be investigated). I will submit TestExpectations once the tests are imported BUG=705252 TEST=None ========== to ========== [LayoutTests] No longer skip importing payment-request tests from WPT Tests will be imported at the next auto-import From running locally, there are 6 that fail (to be investigated). I will submit TestExpectations once the tests are imported BUG=705252 TEST=None R=foolip@chromium.org Review-Url: https://codereview.chromium.org/2776913002 . Cr-Commit-Position: refs/heads/master@{#460378} Committed: https://chromium.googlesource.com/chromium/src/+/57a65c934af9dce077f720e6fc8a... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 57a65c934af9dce077f720e6fc8a7579de79924f (presubmit successful).
Message was sent while issue was closed.
On 2017/03/29 13:33:54, Mathieu wrote: > On 2017/03/29 09:10:07, foolip_UTC9 wrote: > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > > has some advice for how to do this, and it's to just change > > W3CImportExpectations and then wait for an import. So this LGTM with the > > TestExpectations changes reverted. Once the import is done, you can see what > > failing tests were imported. > > Sounds good! I will check the import. Does it happen often? The goal is once daily, in practice it gets stuck once in a while because automatically updating all expectations is a fairly tricky problem. And right now it'll be delayed by the fact that csswg-test was just merged into web-platform-tests and will require some manual tweaking. But assume it's within a few days and ask qyearsley@ if you've waiting for longer than you like :)
Message was sent while issue was closed.
On 2017/03/29 14:22:32, foolip_UTC9 wrote: > On 2017/03/29 13:33:54, Mathieu wrote: > > On 2017/03/29 09:10:07, foolip_UTC9 wrote: > > > > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > > > has some advice for how to do this, and it's to just change > > > W3CImportExpectations and then wait for an import. So this LGTM with the > > > TestExpectations changes reverted. Once the import is done, you can see what > > > failing tests were imported. > > > > Sounds good! I will check the import. Does it happen often? > > The goal is once daily, in practice it gets stuck once in a while because > automatically updating all expectations is a fairly tricky problem. And right > now it'll be delayed by the fact that csswg-test was just merged into > web-platform-tests and will require some manual tweaking. But assume it's within > a few days and ask qyearsley@ if you've waiting for longer than you like :) Should we warn the sheriffs? I fully expect 6/13 tests to fail... For now I've warned today's sheriff and I have the TestExpectations change ready to go: https://codereview.chromium.org/2776933008/
Message was sent while issue was closed.
There should be no need to warn sheriffs, because there shouldn't ever be any "real" test failures. When the import script updates TestExpectations, failure will be expected, and thus considered a pass. You shouldn't need to land https://codereview.chromium.org/2776933008/ if the import script does its thing right. On Wed, Mar 29, 2017 at 11:24 PM <mathp@chromium.org> wrote: > On 2017/03/29 14:22:32, foolip_UTC9 wrote: > > On 2017/03/29 13:33:54, Mathieu wrote: > > > On 2017/03/29 09:10:07, foolip_UTC9 wrote: > > > > > > > > > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > > > > has some advice for how to do this, and it's to just change > > > > W3CImportExpectations and then wait for an import. So this LGTM with > the > > > > TestExpectations changes reverted. Once the import is done, you can > see > what > > > > failing tests were imported. > > > > > > Sounds good! I will check the import. Does it happen often? > > > > The goal is once daily, in practice it gets stuck once in a while because > > automatically updating all expectations is a fairly tricky problem. And > right > > now it'll be delayed by the fact that csswg-test was just merged into > > web-platform-tests and will require some manual tweaking. But assume it's > within > > a few days and ask qyearsley@ if you've waiting for longer than you > like :) > > Should we warn the sheriffs? I fully expect 6/13 tests to fail... For now > I've > warned today's sheriff and I have the TestExpectations change ready to go: > https://codereview.chromium.org/2776933008/ > > https://codereview.chromium.org/2776913002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
There should be no need to warn sheriffs, because there shouldn't ever be any "real" test failures. When the import script updates TestExpectations, failure will be expected, and thus considered a pass. You shouldn't need to land https://codereview.chromium.org/2776933008/ if the import script does its thing right. On Wed, Mar 29, 2017 at 11:24 PM <mathp@chromium.org> wrote: > On 2017/03/29 14:22:32, foolip_UTC9 wrote: > > On 2017/03/29 13:33:54, Mathieu wrote: > > > On 2017/03/29 09:10:07, foolip_UTC9 wrote: > > > > > > > > > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > > > > has some advice for how to do this, and it's to just change > > > > W3CImportExpectations and then wait for an import. So this LGTM with > the > > > > TestExpectations changes reverted. Once the import is done, you can > see > what > > > > failing tests were imported. > > > > > > Sounds good! I will check the import. Does it happen often? > > > > The goal is once daily, in practice it gets stuck once in a while because > > automatically updating all expectations is a fairly tricky problem. And > right > > now it'll be delayed by the fact that csswg-test was just merged into > > web-platform-tests and will require some manual tweaking. But assume it's > within > > a few days and ask qyearsley@ if you've waiting for longer than you > like :) > > Should we warn the sheriffs? I fully expect 6/13 tests to fail... For now > I've > warned today's sheriff and I have the TestExpectations change ready to go: > https://codereview.chromium.org/2776933008/ > > https://codereview.chromium.org/2776913002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
