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

OIDC: unit test ServerPickerDialog #11019

Merged
merged 8 commits into from
May 31, 2023

Conversation

kerryarchibald
Copy link
Contributor

@kerryarchibald kerryarchibald commented May 31, 2023

Unit test before making changes
For element-hq/element-web#25472

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@kerryarchibald kerryarchibald added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label May 31, 2023

fireEvent.click(screen.getByText("Continue"));

// serverpicker still validates the 'other homeserver' field on submit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logged a bug for this element-hq/element-web#25488

@@ -202,6 +202,7 @@ export default class ServerPickerDialog extends React.PureComponent<IProps, ISta
value="true"
checked={this.state.defaultChosen}
onChange={this.onDefaultChosen}
aria-label={_t("Default homeserver")}
Copy link
Member

@t3chguy t3chguy May 31, 2023

Choose a reason for hiding this comment

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

I'm not sure this is right, this is likely to clobber the reading of any child text like defaultServerName, why is this change in a PR titled unit testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I wanted an easy way to select this for testing and the aria-label seemed harmless. Changed it to a testid.

@kerryarchibald kerryarchibald added this pull request to the merge queue May 31, 2023
Merged via the queue into develop with commit a28f44a May 31, 2023
73 checks passed
@kerryarchibald kerryarchibald deleted the kerry/25472/test-ServerPickerDialog branch May 31, 2023 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants