-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Add test for AutofillGroup api example #156439
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
Add test for AutofillGroup api example #156439
Conversation
b77893b to
997817c
Compare
997817c to
16b8a54
Compare
|
@bleroux can you check it,please? |
bleroux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
See my comment about the formatting.
Also can you add more checks to verify all TextField's autofillHints value? (Reading the sample code, I see some have AutofillHints.streetAddressLine2 and also creditCardNumber, creditCardSecurityCode and telephoneNumber.
Because this example is related to the AutofillGroup feature, it probably would make sense to check the number of AutofilGroup widgets created (similarly to how you checked the number of TextField).
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
bleroux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments before merging:
- consider adding a check for shippingAddress2, this way all the 7 text fields will be verified.
- small typos, see my specific comments.
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
|
I accidentally remove the test for shipping address 2, but it's already fixed, thank you @bleroux |
bleroux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for your contribution 🙏
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
Co-authored-by: Bruno Leroux <bruno.leroux@gmail.com>
TahaTesser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
@bleroux can be merged?🫡 |
Write Tests for API Examples of
autoffil_groupas part of #130459This are tests of snippets used in AutofillGroup class
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.