Skip to content

[Payment Request] Add Web Platform Tests for hasEnrolledInstrument().#15306

Merged
chromium-wpt-export-bot merged 1 commit intomasterfrom
chromium-export-cl-1461313
Feb 9, 2019
Merged

[Payment Request] Add Web Platform Tests for hasEnrolledInstrument().#15306
chromium-wpt-export-bot merged 1 commit intomasterfrom
chromium-export-cl-1461313

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Feb 8, 2019

The failing expectations are required because hasEnrolledInstrument()
is not yet turned on by default in Blink. These tests pass when running
Chrome with --enable-features=PaymentRequestHasEnrolledInstrument.

Bug: 915907
Change-Id: I2189bfedad813e7942c7ddbd44aac78d1b895a30
Reviewed-on: https://chromium-review.googlesource.com/c/1461313
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630578}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

The failing expectations are required because hasEnrolledInstrument()
is not yet turned on by default in Blink. These tests pass when running
Chrome with --enable-features=PaymentRequestHasEnrolledInstrument.

Bug: 915907
Change-Id: I2189bfedad813e7942c7ddbd44aac78d1b895a30
Reviewed-on: https://chromium-review.googlesource.com/c/1461313
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630578}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit a7498dd into master Feb 9, 2019
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-1461313 branch February 9, 2019 01:02
@marcoscaceres
Copy link
Contributor

@foolip something went wrong in the process here. Seems none of the usual reviewers were assigned and this got auto merged?

@foolip
Copy link
Member

foolip commented Feb 15, 2019

@marcoscaceres this was reviewed in https://chromium-review.googlesource.com/c/chromium/src/+/1461313 and this is an automatic export of the change. Gecko has the same setup maintained by @jgraham , in fact ours was inspired by that.

Is there a problem with the tests? I do expect the automatic nature of this to occasionally cause problems as the regular reviewers aren't notified, perhaps this is such a case?

@marcoscaceres
Copy link
Contributor

It’s not a big problem, but sometimes things land that are not yet in specs. Perhaps the policy should be that this be used for bug fixes and refinements, but not for features that haven’t become part of the spec yet... I guess we can also use one of the existing file extensions/postfixes... -tentative or whatever it is.

@marcoscaceres
Copy link
Contributor

(Also understand this is annoying because developing a feature with a good set of tests is obviously ideal 🤔)

@foolip
Copy link
Member

foolip commented Feb 15, 2019

Yes, .tentative. in the filename is the way to do it, documented in https://web-platform-tests.org/writing-tests/file-names.html.

When you see non-standard things landing, I'd suggest pinging the authors in the issue asking them to rename. If you get no reaction or can't find their GitHub handle, ping me and I'll try to help.

@marcoscaceres
Copy link
Contributor

The problem imho is that no human was notified at all once this hit the wpt repo - I only discovered this by accident. Ideally, I’d like to be notified of any and all tests relating to payments (but without subscribing to the whole wpt repo).

@foolip
Copy link
Member

foolip commented Feb 21, 2019

@marcoscaceres I've filed web-platform-tests/wpt-pr-bot#65 about that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants