Skip to content

Conversation

@VirginiaBalseiro
Copy link
Contributor

This should should at least partially address some of the flakiness issues.

  • replace deprecated waitForNavigation with waitForURL

  • use selectors that work across browsers/devices

- replace deprecated waitForNavigation with waitForURL

- use selectors that work across browsers/devices
@VirginiaBalseiro VirginiaBalseiro requested a review from a team as a code owner July 26, 2023 10:51
Comment on lines 39 to 42
// It is important to call waitForURL before click to set up waiting.
this.page.waitForURL("https://login.inrupt.com/openid"),
// Clicking the link will indirectly cause a navigation.
this.page.getByRole('button', { type: 'Submit' }).click(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for added indentations in the diff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh no, just my formatter I suppose :/

@VirginiaBalseiro VirginiaBalseiro temporarily deployed to ESS PodSpaces July 26, 2023 12:52 — with GitHub Actions Inactive
@VirginiaBalseiro VirginiaBalseiro temporarily deployed to ESS PodSpaces July 26, 2023 12:52 — with GitHub Actions Inactive
@VirginiaBalseiro VirginiaBalseiro temporarily deployed to ESS PodSpaces July 26, 2023 12:52 — with GitHub Actions Inactive
// Clicking the link will indirectly cause a navigation.
this.page.click(".visible-lg [aria-label=submit]"),
// It is important to call waitForURL before click to set up waiting.
this.page.waitForURL("https://login.inrupt.com/openid"),
Copy link
Contributor

Choose a reason for hiding this comment

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

isnt this too specific since we have have other IDP domains for other envs than podspaces that this is used with?

@VirginiaBalseiro VirginiaBalseiro temporarily deployed to ESS PodSpaces July 26, 2023 13:03 — with GitHub Actions Inactive
@VirginiaBalseiro VirginiaBalseiro temporarily deployed to ESS PodSpaces July 26, 2023 13:03 — with GitHub Actions Inactive
@VirginiaBalseiro VirginiaBalseiro temporarily deployed to ESS PodSpaces July 26, 2023 13:03 — with GitHub Actions Inactive
@chelseapinka
Copy link
Contributor

which repos did we test this with?

@VirginiaBalseiro
Copy link
Contributor Author

which repos did we test this with?

I tested on AMI and solid-client-access-grants and everything works fr me, but feel free to test on your end too.

@jeswr jeswr merged commit a52d44d into main Jul 26, 2023
@jeswr jeswr deleted the fix/cognito-page branch July 26, 2023 23:48
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.

3 participants