Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

OIDC: test coverage for error cases in Login.tsx #11073

Merged
merged 3 commits into from
Jun 13, 2023

Conversation

kerryarchibald
Copy link
Contributor

@kerryarchibald kerryarchibald commented Jun 12, 2023

For element-hq/element-web#25468

Add coverage before making changes in 25468

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 Jun 12, 2023
@kerryarchibald kerryarchibald marked this pull request as ready for review June 12, 2023 06:04
@kerryarchibald kerryarchibald requested a review from a team as a code owner June 12, 2023 06:04
Copy link
Contributor

@artcodespace artcodespace left a comment

Choose a reason for hiding this comment

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

Looks good to me, one tiny non-blocking comment

Comment on lines +287 to +299
fetchMock.resetBehavior();
fetchMock.get("https://matrix.org/_matrix/client/versions", {
status: 400,
});
getComponent();
await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));

// error displayed
expect(screen.getByText("Your test-brand is misconfigured")).toBeInTheDocument();
});

it("should reset liveliness error when server config changes", async () => {
fetchMock.resetBehavior();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these .resetBehaviours if they're already in the beforeEach hook?

Not familiar with using the package the mocked fetch comes from, so there could be some quirk I'm unaware of here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found them necessary to reset clear the /versions mock that is set in the beforeEach hook

@kerryarchibald kerryarchibald added this pull request to the merge queue Jun 13, 2023
Merged via the queue into develop with commit f7137b4 Jun 13, 2023
@kerryarchibald kerryarchibald deleted the kerry/25468/test-login branch June 13, 2023 03:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants