Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confirm tests and specs align #779

Closed
17 of 29 tasks
marcoscaceres opened this issue Sep 26, 2018 · 6 comments
Closed
17 of 29 tasks

Confirm tests and specs align #779

marcoscaceres opened this issue Sep 26, 2018 · 6 comments

Comments

@marcoscaceres
Copy link
Member

marcoscaceres commented Sep 26, 2018

We need to make sure that the tests listed in the spec and the actual tests in the test suite align.

We need:

  1. The list of files in WPT's payment request directory.
  2. To be compared to the "data-test" files listed in the spec (example).

Then we can figure out:

  • Are all tests in the test suite in the spec?
  • Are tests listed in the spec that don't actually link to anything?

@Sylvia23 has offered to help us with this 👋

The tests that are in the payment request directory but not in the spec are:

  • MerchantValidationEvent/complete-method.https.html
  • PaymentAddress/attributes-and-toJSON-method-manual.https.html
  • PaymentMethodChangeEvent/methodDetails-attribute.https.html
  • PaymentMethodChangeEvent/methodName-attribute.https.html
  • PaymentRequestUpdateEvent/updateWith-call-immediate-manual.https.html
  • PaymentRequestUpdateEvent/updateWith-method-abort-update-manual.https.html
  • PaymentRequestUpdateEvent/updateWith-state-checks-manual.https.html
  • PaymentRequestUpdateEvent/updatewith-method.https.html
  • PaymentValidationErrors/retry-shows-payer-member-manual.https.html
  • PaymentValidationErrors/retry-shows-shippingAddress-member-manual.https.html
  • payment-response/onpayerdetailchange-attribute-manual.https.html
  • payment-response/onpayerdetailchange-attribute.https.html
  • payment-request-abort-method.https.html
  • payment-request-canmakepayment-method.https.html
  • payment-request-not-exposed.https.worker.js

-The tests that are in the spec but are not present in or are wrongly addressed in the payment directory are:

Line 640:

  • active-document-cross-origin.https.sub.html
  • active-document-same-origin.https.html
  • removing-allowpaymentrequest.https.sub.html
  • setting-allowpaymentrequest-timing.https.sub.html
  • setting-allowpaymentrequest.https.sub.html

Line 904:

  • payment-request-show-method-manual.https.html

Line 910:

  • payment-request-show-method-manual.https.html

Line 1138:

  • payment-request-abort-method-manual.https.html

Line 1197:

  • payment-request-canmakepayment-method-manual.https.html

Line 1300:

  • payment-request/onmerchantvalidation-attribute.https.html

Line 3290:

  • payment-request/PaymentValidationErrors/retry-shows-error-member-manual.https.html

Line 3322:

  • payment-request/payment-response/rejects_if_not_active-manual.https.html

Line 3630:

  • payment-request/payment-response/retry-method-manual.https.html

Line 4177:

  • payment-request-update-event-updatewith-method.https.html
@Sylvia23
Copy link
Contributor

Thank you so much @marcoscaceres for writing down this issue. I would love to help with this issue and report back in a while! Thanks!

Sylvia23 added a commit to Sylvia23/payment-request that referenced this issue Sep 27, 2018
@marcoscaceres marcoscaceres changed the title Confirm test and specs align Confirm tests and specs align Sep 28, 2018
@Sylvia23
Copy link
Contributor

@marcoscaceres The part of the issue - "There are tests in the spec but are not present in or are wrongly addressed in the payment directory" has been fixed with #788 . Please review. Thanks!

@marcoscaceres
Copy link
Member Author

@Sylvia23, great work so far. Could you let us know what’s left to be done?

@Sylvia23
Copy link
Contributor

Thank you so much @marcoscaceres . Now, we are left with adding the extra test suites that are there in the payment request repository but are missing in our spec. So, we need to add those in the spec. I have again checked the test suites allignment after merging #788 . I found that we can now check the boxes for MerchantValidationEvent/complete-method.https.html , payment-request-abort-method.https.html, payment-request-canmakepayment-method.https.html in our checklist because they got resolved automatically by #788 . Rest of the tests still need to be worked upon. I am reading the tests and their attributes to get an idea about what they are about so as to put them in their appropriate position in the spec. Any kind of suggestion is highly welcomed. Thanks!

@ianbjacobs
Copy link
Collaborator

@Sylvia23 and @marcoscaceres, thank you very much for work on this!

@marcoscaceres
Copy link
Member Author

Closing as done. Thanks again, @Sylvia23.

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

No branches or pull requests

3 participants