Closed Bug 1481295 Opened Last year Closed Last year

Enable PaymentRequest in WPT /payment-request/ tests

Categories

(Core :: DOM: Web Payments, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mrbkap, Assigned: edenchuang)

References

Details

(Whiteboard: [webpayments-reserve])

Attachments

(1 file, 4 obsolete files)

I noticed looking at another bug that we don't set the pref for idlharness.https.window.js.ini, so it fails. It seems like we might as run the test with the pref set.
Attachment #8998006 - Attachment description: Bug 1481295 - Set the pref so we see what's acutally failing. → Bug 1481295 - Set the pref so we see what's actually failing.
Priority: -- → P2
Attachment #8998006 - Flags: review?(echuang)
Comment on attachment 8998006 [details]
Bug 1481295 - Set the pref so we see what's actually failing.

Eden Chuang[:edenchuang] has approved the revision.
Attachment #8998006 - Flags: review+
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f5873bdd1241
Set the pref so we see what's actually failing. r=edenchuang
Backed out changeset f5873bdd1241 (Bug 1481295) for idlharness.https.window.html failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f5873bdd1241fc05a1839e78ac1b24774eb7c58a

Backout link: https://hg.mozilla.org/integration/autoland/rev/81b7c0a6c98663406ab8321f9f686837761dfbae

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=192804564&repo=autoland&lineNumber=18598
[task 2018-08-08T16:06:38.504Z] 16:06:38     INFO - TEST-START | /payment-request/idlharness.https.window.html
[task 2018-08-08T16:06:38.605Z] 16:06:38     INFO - Setting pref dom.payments.request.enabled (true)
[task 2018-08-08T16:06:39.540Z] 16:06:39     INFO - PID 8682 | ++DOCSHELL 0xe043f000 == 9 [pid = 8682] [id = {b96de763-6e8c-45b4-910c-bfbf5274b869}]
[task 2018-08-08T16:06:39.540Z] 16:06:39     INFO - PID 8682 | ++DOMWINDOW == 27 (0xdfe94160) [pid = 8682] [serial = 27] [outer = (nil)]
[task 2018-08-08T16:06:39.556Z] 16:06:39     INFO - PID 8682 | ++DOMWINDOW == 28 (0xe0440c00) [pid = 8682] [serial = 28] [outer = 0xdfe94160]
[task 2018-08-08T16:06:39.653Z] 16:06:39     INFO - PID 8682 | ++DOMWINDOW == 29 (0xe043fc00) [pid = 8682] [serial = 29] [outer = 0xdfe94160]
[task 2018-08-08T16:06:39.762Z] 16:06:39     INFO - PID 8682 | [8682, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /builds/worker/workspace/build/src/parser/html/nsHtml5StreamParser.cpp, line 1015
[task 2018-08-08T16:06:41.582Z] 16:06:41     INFO - 
[task 2018-08-08T16:06:41.582Z] 16:06:41     INFO - TEST-PASS | /payment-request/idlharness.https.window.html | Setup for Payment Request API IDL tests. 
[task 2018-08-08T16:06:41.583Z] 16:06:41     INFO - TEST-UNEXPECTED-FAIL | /payment-request/idlharness.https.window.html | PaymentRequest interface: existence and properties of interface object - assert_own_property: self does not have own property "PaymentRequest" expected property "PaymentRequest" missing
[task 2018-08-08T16:06:41.585Z] 16:06:41     INFO - IdlInterface.prototype.test_self/<@https://web-platform.test:8443/resources/idlharness.js:1506:9
[task 2018-08-08T16:06:41.586Z] 16:06:41     INFO - Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1538:20
[task 2018-08-08T16:06:41.588Z] 16:06:41     INFO - test@https://web-platform.test:8443/resources/testharness.js:545:9
[task 2018-08-08T16:06:41.590Z] 16:06:41     INFO - self.subsetTestByKey@https://web-platform.test:8443/resources/idlharness.js:54:23
[task 2018-08-08T16:06:41.591Z] 16:06:41     INFO - IdlInterface.prototype.test_self@https://web-platform.test:8443/resources/idlharness.js:1486:5
[task 2018-08-08T16:06:41.593Z] 16:06:41     INFO - IdlInterface.prototype.test@https://web-platform.test:8443/resources/idlharness.js:1470:9
[task 2018-08-08T16:06:41.595Z] 16:06:41     INFO - IdlArray.prototype.test@https://web-platform.test:8443/resources/idlharness.js:857:9
[task 2018-08-08T16:06:41.597Z] 16:06:41     INFO - idl_test/</<@https://web-platform.test:8443/resources/idlharness.js:3226:21
[task 2018-08-08T16:06:41.599Z] 16:06:41     INFO - Async*idl_test/<@https://web-platform.test:8443/resources/idlharness.js:3205:16
[task 2018-08-08T16:06:41.600Z] 16:06:41     INFO - Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1538:20
[task 2018-08-08T16:06:41.601Z] 16:06:41     INFO - promise_test/tests.promise_tests<@https://web-platform.test:8443/resources/testharness.js:577:27
[task 2018-08-08T16:06:41.602Z] 16:06:41     INFO - promise callback*promise_test@https://web-platform.test:8443/resources/testharness.js:573:31
[task 2018-08-08T16:06:41.602Z] 16:06:41     INFO - idl_test@https://web-platform.test:8443/resources/idlharness.js:3200:12
[task 2018-08-08T16:06:41.602Z] 16:06:41     INFO - @https://web-platform.test:8443/payment-request/idlharness.https.window.js:8:1
[task 2018-08-08T16:06:41.603Z] 16:06:41     INFO - 
[task 2018-08-08T16:06:41.603Z] 16:06:41     INFO - TEST-UNEXPECTED-FAIL | /payment-request/idlharness.https.window.html | PaymentRequest interface object length - assert_own_property: self does not have own property "PaymentRequest" expected property "PaymentRequest" missing
:asuth on your radar
Flags: needinfo?(bugmail)
Whiteboard: [webpayments][triage]
Status: NEW → ASSIGNED
Flags: qe-verify-
Priority: P2 → P1
Whiteboard: [webpayments][triage] → [webpayments-reserve]
Assignee: mrbkap → echuang
Attachment #8998006 - Flags: review+
Priority: P1 → P2
For assigned bugs being actively worked on we put the priority at P1.
Priority: P2 → P1
Attachment #8998006 - Attachment is obsolete: true
Attachment #8998006 - Flags: review?(echuang)
Attachment #9012191 - Flags: review?(amarchesini)
Comment on attachment 9012191 [details] [diff] [review]
Set Pref dom.payments.request.enabled on for running wp-tests with expected Pass results on tryserver.

Review of attachment 9012191 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/meta/payment-request/MerchantValidationEvent/complete-method.https.html.ini
@@ +1,1 @@
>  [complete-method.https.html]

Remove this file completely.

::: testing/web-platform/meta/payment-request/MerchantValidationEvent/constructor.https.html.ini
@@ +1,1 @@
>  [constructor.https.html]

All of these files must be removed.
Attachment #9012191 - Flags: review?(amarchesini) → review-
Comment on attachment 9012191 [details] [diff] [review]
Set Pref dom.payments.request.enabled on for running wp-tests with expected Pass results on tryserver.

Review of attachment 9012191 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing this Eden. I'm super excited that all these will finally be enabled! 

Just two small things related to "dom.payments.request.user_interaction_required".

::: testing/web-platform/meta/payment-request/payment-request-abort-method.https.html.ini
@@ +1,2 @@
>  [payment-request-abort-method.https.html]
> +  prefs: [dom.payments.request.user_interaction_required:false]

Please remove this, or leave it as expect FAIL. That will remind us to fix this later.

::: testing/web-platform/meta/payment-request/payment-request-canmakepayment-method.https.html.ini
@@ +1,2 @@
>  [payment-request-canmakepayment-method.https.html]
> +  prefs: [dom.payments.request.user_interaction_required:false]

Same as the other. Let this one FAIL.
Attachment #9012191 - Flags: feedback+
Comment on attachment 9012191 [details] [diff] [review]
Set Pref dom.payments.request.enabled on for running wp-tests with expected Pass results on tryserver.

Review of attachment 9012191 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/meta/payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html.ini
@@ +1,2 @@
>  [onpayerdetailchange-attribute.manual.https.html]
> +  expected: TIMEOUT

Note, I fixed this this in https://bugzilla.mozilla.org/show_bug.cgi?id=1493110

The file was incorrectly named. I think this filed can be deleted (or probably has been deleted upstream). The new file should not timeout anymore :)
Comment on attachment 9012191 [details] [diff] [review]
Set Pref dom.payments.request.enabled on for running wp-tests with expected Pass results on tryserver.

Review of attachment 9012191 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/meta/payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html.ini
@@ +1,2 @@
>  [onpayerdetailchange-attribute.manual.https.html]
> +  expected: TIMEOUT

Note, I fixed this this in https://bugzilla.mozilla.org/show_bug.cgi?id=1493110

The file was incorrectly named. I think this filed can be deleted (or probably has been deleted upstream). The new file should not timeout anymore :)
update patch according to comment 9 and 10.
Attachment #9012510 - Flags: review?(amarchesini)
(In reply to Marcos Caceres [:marcosc] from comment #10)
> Comment on attachment 9012191 [details] [diff] [review]
> Set Pref dom.payments.request.enabled on for running wp-tests with expected
> Pass results on tryserver.
> 
> Review of attachment 9012191 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for doing this Eden. I'm super excited that all these will finally be
> enabled! 
> 
> Just two small things related to
> "dom.payments.request.user_interaction_required".
> 
> :::
> testing/web-platform/meta/payment-request/payment-request-abort-method.https.
> html.ini
> @@ +1,2 @@
> >  [payment-request-abort-method.https.html]
> > +  prefs: [dom.payments.request.user_interaction_required:false]
> 
> Please remove this, or leave it as expect FAIL. That will remind us to fix
> this later.
> 
> :::
> testing/web-platform/meta/payment-request/payment-request-canmakepayment-
> method.https.html.ini
> @@ +1,2 @@
> >  [payment-request-canmakepayment-method.https.html]
> > +  prefs: [dom.payments.request.user_interaction_required:false]
> 
> Same as the other. Let this one FAIL.

In fact, I file a following bug for it. https://bugzilla.mozilla.org/show_bug.cgi?id=1494340
but I still update the patch according to your comment. Let tests be expected FAIL.
(In reply to Marcos Caceres [:marcosc] from comment #12)
> Comment on attachment 9012191 [details] [diff] [review]
> Set Pref dom.payments.request.enabled on for running wp-tests with expected
> Pass results on tryserver.
> 
> Review of attachment 9012191 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> :::
> testing/web-platform/meta/payment-request/payment-response/
> onpayerdetailchange-attribute.manual.https.html.ini
> @@ +1,2 @@
> >  [onpayerdetailchange-attribute.manual.https.html]
> > +  expected: TIMEOUT
> 
> Note, I fixed this this in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1493110
> 
> The file was incorrectly named. I think this filed can be deleted (or
> probably has been deleted upstream). The new file should not timeout anymore
> :)

But I still get TIMEOUT on tryserver. I will double check on tryserver before I set checkin-needed.
I guess we should get the expected result, even the .ini file exists, because the test should not be run automatically.
So now, I will keep it on the patch, and if it still needed when I checkin, I will file a following bug for it.
(In reply to Eden Chuang[:edenchuang] from comment #15)
> But I still get TIMEOUT on tryserver. I will double check on tryserver
> before I set checkin-needed.

Yeah, that's odd. If it's the same file, then that's the problem. That file should have been deleted from upstream WPT. 

> I guess we should get the expected result, even the .ini file exists,
> because the test should not be run automatically.
> So now, I will keep it on the patch, and if it still needed when I checkin,
> I will file a following bug for it.

You can leave it as expecting TIMEOUT, I would guess... and it should self heal once the metadata gets regenerated.
Comment on attachment 9012510 [details] [diff] [review]
Set Pref dom.payments.request.enabled on for running wp-tests with expected Pass results on tryserver.

Review of attachment 9012510 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/meta/payment-request/payment-request-id-attribute.https.html.ini
@@ +2,3 @@
>    [PaymentRequest's id attribute must be a UUID when PaymentDetailsInit.id is missing]
> +    expected:
> +      if os == "mac": FAIL

why this? we need a comment a bug to fix the issue.
Attachment #9012510 - Flags: review?(amarchesini) → review+
Comment on attachment 9012510 [details] [diff] [review]
Set Pref dom.payments.request.enabled on for running wp-tests with expected Pass results on tryserver.

Review of attachment 9012510 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/meta/payment-request/payment-request-id-attribute.https.html.ini
@@ +2,3 @@
>    [PaymentRequest's id attribute must be a UUID when PaymentDetailsInit.id is missing]
> +    expected:
> +      if os == "mac": FAIL

This is weird... the RegExp passes in all other browsers. We might need to get the JS folks to look at this?
To be clear, I confirmed that it's indeed failing in Mac OS - but I didn't check other OSs.
(In reply to Marcos Caceres [:marcosc] from comment #19)
> To be clear, I confirmed that it's indeed failing in Mac OS - but I didn't
> check other OSs.

Yes only on MacOS, but pass in other OSs.
I would like to comment it on .ini and file a bug to figure it out.
Fixing on WTP side by making the RegExp a little bit less strict: 
https://github.com/web-platform-tests/wpt/pull/13258
Keywords: checkin-needed
Could not land patch, received this error message when trying to apply the patch: "unable to find 'testing/web-platform/meta/payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html.ini' for patching"

:edenchuang could you please take a look?
Flags: needinfo?(echuang)
That’s the file that got renamed from .manual to -manual.
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/77168ec151a9
set prefs for PaymentRequest web-platform-tests. r=baku.
Keywords: checkin-needed
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d45a7a0f6f29
Follow-up to revert mistaken PaymentAddress.languageCode change. r=bustage
Just want to record what the actual issue is... when the UUID is generated on MacOS, the third component's (time_hi_and_version) is not returning what's expected by the UUID spec: the first character should be a digit between 1-4. It might just have been coincidence that we didn't hit this on other platforms.

Looking briefly at nsID.{cpp, h} [1,2], I don't see anything platform specific there - but [2] does not claim to be a fully conforming implementation of UUID. It states: "A "unique identifier". This is modeled after OSF DCE UUIDs." 

Should we need info someone about this - maybe about making us more conforming? The Payment spec requires UUID. However, not providing a fully conforming UUID should not be a big deal in practice (famous last words, specially if there are systems/databases that do validate UUIDs). 

[1] https://searchfox.org/mozilla-central/source/xpcom/base/nsID.cpp
[2] https://searchfox.org/mozilla-central/source/xpcom/base/nsID.h#17
It might be good to reference your comment in bug 1494920.
Depends on: 1494920
Fix the test fail on PaymentAddress updating in web-platform-tests.
Attachment #9012974 - Attachment is obsolete: true
Flags: needinfo?(echuang)
Attachment #9013077 - Flags: review+
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d799420c9b97
set prefs for PaymentRequest web-platform-tests. r=baku.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d799420c9b97
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Summary: Update idlharness.https.window.js metadata → Enable PaymentRequest in WPT /payment-request/ tests
You need to log in before you can comment on or make changes to this bug.