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 Sign Up tests #606

Merged
merged 12 commits into from
Nov 3, 2021
Merged

Confirm Sign Up tests #606

merged 12 commits into from
Nov 3, 2021

Conversation

ericclemmons
Copy link
Contributor

@ericclemmons ericclemmons commented Nov 2, 2021

Issue #, if available: Fixes #569

Screen Shot 2021-11-01 at 2 52 25 PM

Description of changes:

  • Update confirm-sign-up.feature tests to run against @angular @react @vue
  • Introduce new I mock {string} with fixture {string} behavior
  • Expose window.Amplify.* for the new behavior
  • Fix React tests (@ericclemmons)
  • Fix Vue tests (@ErikCH)
  • Fix Angualr tests (@wlee221)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@changeset-bot
Copy link

changeset-bot bot commented Nov 2, 2021

⚠️ No Changeset found

Latest commit: 38a6a4a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor Author

@ericclemmons ericclemmons left a comment

Choose a reason for hiding this comment

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

@ErikCH I've made a small change for Vue, but I'm noticing that Cypress is having issues typing into the Confirmation Code field. Can you help debug these?
Screen Shot 2021-11-02 at 1 37 54 PM
Screen Shot 2021-11-02 at 1 38 06 PM
Screen Shot 2021-11-02 at 1 38 29 PM

Copy link
Contributor Author

@ericclemmons ericclemmons left a comment

Choose a reason for hiding this comment

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

@wlee221 I'm seeing similar issues as Vue when I run the e2e tests – typing into the Confirmation Code field doesn't seem to work:
Screen Shot 2021-11-02 at 1 50 28 PM
Screen Shot 2021-11-02 at 2 02 23 PM

@wlee221
Copy link
Contributor

wlee221 commented Nov 2, 2021

@ericclemmons Ah, I think I know what the problem is!

  1. you should preferably use
- cy.findByLabelText('Confirmation Code').type('invalidcode');
+ cy.findInputField('Confirmation Code').type('invalidcode');

which searches for accessible textbox instead of the label.

  1. In Vue and Angular, code field has type="number", so dom-testing-library thinks it's a spinbutton instead of a textbox.

I think we should make type consistent first, then we can figure out the tests.

@ericclemmons
Copy link
Contributor Author

@wlee221 I attempted that change locally, but it didn't seem to resolve it. I agree, the type should be consistent and as a text. (Though it's a number, the spin controls don't help because they're not incrementing like a quantity)

@wlee221 wlee221 temporarily deployed to ci November 2, 2021 20:39 Inactive
@wlee221 wlee221 temporarily deployed to ci November 2, 2021 20:39 Inactive
@wlee221 wlee221 temporarily deployed to ci November 2, 2021 20:39 Inactive
@ericclemmons ericclemmons merged commit 6a56a50 into main Nov 3, 2021
@ericclemmons ericclemmons deleted the confirm-sign-up-e2e branch November 3, 2021 15:33
thaddmt pushed a commit that referenced this pull request Apr 7, 2023
Sync amplify-ui to amplify-ui-staging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feedback] First impression on the new authenticator / UI.
2 participants