Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2623003002:
Added idlharness test for webauth (Closed)
|
Created:
3 years, 11 months ago by kpaulhamus Modified:
3 years, 10 months ago Reviewers:
foolip CC:
chromium-reviews, blink-reviews, juanlang (chromium.org), aczeskis Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdded idlharness test for webauth
Needs the "-expected.txt" file due to issue with DOM prototypes identifiers:
See https://crbug.com/643712.
BUG=664630
Review-Url: https://codereview.chromium.org/2623003002
Cr-Commit-Position: refs/heads/master@{#449384}
Committed: https://chromium.googlesource.com/chromium/src/+/6adcf35d6ca7f2c4cfb6536880e996fc877a50d1
Patch Set 1 #Patch Set 2 : Removed commented-out line #
Total comments: 6
Patch Set 3 : Moved idls from untested to tested; updated expected file #
Messages
Total messages: 32 (22 generated)
Description was changed from ========== Added idlharness test for webauth. Needs the "-expected.txt" file due to issue with DOM prototypes identifiers: See https://crbug.com/643712. BUG=664630 ========== to ========== Added idlharness test for webauth. Needs the "-expected.txt" file due to issue with DOM prototypes identifiers: See https://crbug.com/643712. BUG=664630 ==========
The CQ bit was checked by kpaulhamus@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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by kpaulhamus@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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by kpaulhamus@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...
kpaulhamus@chromium.org changed reviewers: + foolip@chromium.org
Hi, do you mind reviewing this idlharness test? (It's pretty basic). Thanks.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/linux/webauth/idl-expected.txt (right): https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/linux/webauth/idl-expected.txt:1: This is a testharness.js-based test. Move this to third_party/WebKit/LayoutTests/webauth/idl-expected.txt to resolve the bot failures. https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webauth/idl.html (right): https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webauth/idl.html:6: <script type="text/plain" id="untested"> Why are these bits left untested? https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webauth/idl.html:84: Promise < ScopedCredentialInfo > makeCredential ( Filed https://github.com/w3c/webauthn/issues/335 because I didn't like this style.
Description was changed from ========== Added idlharness test for webauth. Needs the "-expected.txt" file due to issue with DOM prototypes identifiers: See https://crbug.com/643712. BUG=664630 ========== to ========== Added idlharness test for webauth Needs the "-expected.txt" file due to issue with DOM prototypes identifiers: See https://crbug.com/643712. BUG=664630 ==========
I changed the description. The first line must be a subject that spans only one line.
Hmm, https://github.com/w3c/web-platform-tests/tree/master/webauthn already exists, have you looked into importing that instead? See https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat...
The CQ bit was checked by kpaulhamus@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...
On 2017/01/30 15:24:09, foolip_UTC7 wrote: > Hmm, https://github.com/w3c/web-platform-tests/tree/master/webauthn already > exists, have you looked into importing that instead? See > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... Yeah, those tests aren't fully fleshed out. For ease of development, I also pre-emptively made some changes to the idl here that haven't formally made it into the spec *yet* but are on their way. I was planning on working with Adam on the platform tests when the spec stabilized a bit more.
https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/linux/webauth/idl-expected.txt (right): https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/linux/webauth/idl-expected.txt:1: This is a testharness.js-based test. On 2017/01/30 15:19:30, foolip_UTC7 wrote: > Move this to third_party/WebKit/LayoutTests/webauth/idl-expected.txt to resolve > the bot failures. Ah! I couldn't figure them out. Thanks! https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webauth/idl.html (right): https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webauth/idl.html:6: <script type="text/plain" id="untested"> On 2017/01/30 15:19:30, foolip_UTC7 wrote: > Why are these bits left untested? No real reason, apparently. Moved everything from untested to tested. https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webauth/idl.html:84: Promise < ScopedCredentialInfo > makeCredential ( On 2017/01/30 15:19:30, foolip_UTC7 wrote: > Filed https://github.com/w3c/webauthn/issues/335 because I didn't like this > style. Ack. I'll update the code when the issue is resolved.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/01/30 19:08:35, kpaulhamus wrote: > https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... > File third_party/WebKit/LayoutTests/platform/linux/webauth/idl-expected.txt > (right): > > https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/platform/linux/webauth/idl-expected.txt:1: This > is a testharness.js-based test. > On 2017/01/30 15:19:30, foolip_UTC7 wrote: > > Move this to third_party/WebKit/LayoutTests/webauth/idl-expected.txt to > resolve > > the bot failures. > > Ah! I couldn't figure them out. Thanks! > > https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... > File third_party/WebKit/LayoutTests/webauth/idl.html (right): > > https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/webauth/idl.html:6: <script type="text/plain" > id="untested"> > On 2017/01/30 15:19:30, foolip_UTC7 wrote: > > Why are these bits left untested? > > No real reason, apparently. Moved everything from untested to tested. > > https://codereview.chromium.org/2623003002/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/webauth/idl.html:84: Promise < > ScopedCredentialInfo > makeCredential ( > On 2017/01/30 15:19:30, foolip_UTC7 wrote: > > Filed https://github.com/w3c/webauthn/issues/335 because I didn't like this > > style. > > Ack. I'll update the code when the issue is resolved. To clarify -> I'll update in a later CL.
On 2017/01/30 19:08:19, kpaulhamus wrote: > On 2017/01/30 15:24:09, foolip_UTC7 wrote: > > Hmm, https://github.com/w3c/web-platform-tests/tree/master/webauthn already > > exists, have you looked into importing that instead? See > > > https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... > > Yeah, those tests aren't fully fleshed out. For ease of development, I also > pre-emptively made some changes to the idl here that haven't formally made it > into the spec *yet* but are on their way. I was planning on working with Adam on > the platform tests when the spec stabilized a bit more. So, do you want to have duplicate IDL tests for this feature? LGTM if so, but I'm keen to hear what it would take to write tests in web-platform-tests only. It sounds like you want to implement some things before they are spec'd, is that the core of the problem? If so, would it all be solved it you had a place in web-platform-tests where you could check such forward-looking tests that are then moved to the proper location once confirmed to match the spec? Other ideas?
The CQ bit was checked by kpaulhamus@chromium.org
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": 40001, "attempt_start_ts": 1486667312247790,
"parent_rev": "e892c6f828b8b5da963f5c184cbda5d7841dfc5b", "commit_rev":
"6adcf35d6ca7f2c4cfb6536880e996fc877a50d1"}
Message was sent while issue was closed.
Description was changed from ========== Added idlharness test for webauth Needs the "-expected.txt" file due to issue with DOM prototypes identifiers: See https://crbug.com/643712. BUG=664630 ========== to ========== Added idlharness test for webauth Needs the "-expected.txt" file due to issue with DOM prototypes identifiers: See https://crbug.com/643712. BUG=664630 Review-Url: https://codereview.chromium.org/2623003002 Cr-Commit-Position: refs/heads/master@{#449384} Committed: https://chromium.googlesource.com/chromium/src/+/6adcf35d6ca7f2c4cfb6536880e9... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/6adcf35d6ca7f2c4cfb6536880e9... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
