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

Split the read-receipt tests into logical units #11649

Merged
merged 3 commits into from
Sep 22, 2023

Conversation

andybalaam
Copy link
Member

@andybalaam andybalaam commented Sep 21, 2023

Part of element-hq/element-web#25449

This should prevent them being super-slow on CI, and crashing Chrome.


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

@andybalaam andybalaam added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Sep 21, 2023
@andybalaam andybalaam force-pushed the andybalaam/split-receipt-tests-into-4 branch from 25cf817 to 82e65c8 Compare September 21, 2023 23:52
@andybalaam andybalaam marked this pull request as ready for review September 22, 2023 00:30
@andybalaam andybalaam requested a review from a team as a code owner September 22, 2023 00:30
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

It's hard to do a comprehensive review of this, but the general principle seems sensible.

cypress/e2e/read-receipts/missing-referents.spec.ts Outdated Show resolved Hide resolved
*/

/*
* # High Level Read Receipt Tests
Copy link
Member

Choose a reason for hiding this comment

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

also, presumably only high-level.spec.ts are "High Level" tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

I still consider all of these high level. I avoided renaming the "high-level" file as "misc" because I didn't like that name and it made it easer for me to preserve my later PRs.

I am kind of hoping that when we flesh out more tests inside that file they will earn their own separate files with good names.

Copy link
Member

Choose a reason for hiding this comment

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

ok. It might be good to create a high-level subdirectory then?

@germain-gg germain-gg removed their request for review September 22, 2023 08:56
@andybalaam andybalaam added this pull request to the merge queue Sep 22, 2023
@andybalaam andybalaam removed this pull request from the merge queue due to a manual request Sep 22, 2023
@andybalaam andybalaam added this pull request to the merge queue Sep 22, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 22, 2023
@andybalaam andybalaam added this pull request to the merge queue Sep 22, 2023
Merged via the queue into develop with commit 1c2d604 Sep 22, 2023
19 checks passed
@andybalaam andybalaam deleted the andybalaam/split-receipt-tests-into-4 branch September 22, 2023 14:07
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