WICG / digital-credentials

Digital Credentials, like driver's licenses
https://wicg.github.io/digital-credentials/
Other
66 stars 8 forks source link

Web Platform Tests: refactoring digitial-credentials.tentative.https.html #118

Open samuelgoto opened 3 weeks ago

samuelgoto commented 3 weeks ago

Posting this on behalf of @marcoscaceres, who posted this on slack:

@samuelgoto , do you mind if I refactor digitial-credentials.tentative.https.html ? Some things in there are either not specified yet, add indirection (making tests harder to debug), or might not work in the long term (e.g., requestIdentityWithActivation()). The third party iframe stuff does a bit more work than it probably should, by calling requestIdentityWithActivation()... there are cases where we want to test third party iframes without user activation, so we should do that explicitly as part of tests. There's other tests that won't work... like ones that request an object and expect to get a response. As it requires the user to actually click stuff in the credential sheet UI. (edited) This one also is problematic (though not exactly wrong):

  promise_test(async (t) => {
      const request = {...IdentityRequestWithRequestObject}
      const largeList = [];
      for (let i = 0; i < 1000000; ++i) {
        largeList.push("Value " + i);
      }
      request.digital.providers[0].request.random = largeList;
      await promise_rejects_js(t, TypeError, navigator.identity.get(request));
    }, "navigator.identity.get() API fails when IdentityRequestProvider#request object is too big");

It's just that the limits can't be enforced there... plus if there's adherence to the protocol, "random" would have been dropped on the floor

Another test in there that I think is incorrect ends up throwing a TypeError if there's more than one provider.

marcoscaceres commented 3 weeks ago

Working on refactor of tests... hope to have something up later today.

marcoscaceres commented 3 weeks ago

Sent a PR https://github.com/web-platform-tests/wpt/pull/46642 ... now aligns with the spec.

I have a PR coming for some of the Cred Man integration too. Hopefully by tomorrow.