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

Use Cypress Testing Library - right-panel.spec.ts #10539

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Use Cypress Testing Library - right-panel.spec.ts #10539

merged 1 commit into from
Apr 17, 2023

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Apr 6, 2023

For element-hq/element-web#25033

This PR intends to update right-panel.spec.ts to use Cypress Testing Library.

type: task

Signed-off-by: Suguru Hirahara [email protected]

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.

Sorry, something went wrong.

checkRoomSummaryCard(ROOM_NAME);
});

it("should handle viewing room member", () => {
viewRoomSummaryByName(ROOM_NAME);

cy.get(".mx_RoomSummaryCard_icon_people").click();
// \d represents the number of the room members inside mx_BaseCard_Button_sublabel
cy.findByRole("button", { name: /People \d/ }).click();
Copy link
Contributor Author

@luixxiul luixxiul Apr 11, 2023

Choose a reason for hiding this comment

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

Since there is not another test member on this test, this should be equal to "People 1". I thought searching with a regex pattern would be ok, as this should not be a place to check the room members count, but I am not perfectly convinced that searching "People \d" would be better than "People 1", because the latter is what the user perceives on the actual UI.

I am wondering if creating a dedicated test on this file to check whether room members number is properly counted would be in fact better.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I agree it's better to just assert one thing at a time rather than effectively checking the people number is correct too. The code comment is helpful, thanks!

Verified

This commit was signed with the committer’s verified signature.
luixxiul Suguru Hirahara
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul luixxiul marked this pull request as ready for review April 11, 2023 16:45
@luixxiul luixxiul requested a review from a team as a code owner April 11, 2023 16:45
@luixxiul luixxiul requested review from dbkr and kerryarchibald April 11, 2023 16:45
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Thanks!

checkRoomSummaryCard(ROOM_NAME);
});

it("should handle viewing room member", () => {
viewRoomSummaryByName(ROOM_NAME);

cy.get(".mx_RoomSummaryCard_icon_people").click();
// \d represents the number of the room members inside mx_BaseCard_Button_sublabel
cy.findByRole("button", { name: /People \d/ }).click();
Copy link
Member

Choose a reason for hiding this comment

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

Yes, I agree it's better to just assert one thing at a time rather than effectively checking the people number is correct too. The code comment is helpful, thanks!

@dbkr dbkr added this pull request to the merge queue Apr 17, 2023
@luixxiul
Copy link
Contributor Author

Thanks for the review.

Merged via the queue into matrix-org:develop with commit 4d859a3 Apr 17, 2023
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 Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants