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.

Sign up for GitHub

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

Jump to bottom

[Disabled for Element-R] Flaky cypress test Read receipts > Paging up > After marking room as read, paging up to find old threads that were never read leaves the room read #26341

Closed
andybalaam opened this issue Oct 9, 2023 · 8 comments · Fixed by matrix-org/matrix-react-sdk#11781 or matrix-org/matrix-react-sdk#12007
Closed

[Disabled for Element-R] Flaky cypress test Read receipts > Paging up > After marking room as read, paging up to find old threads that were never read leaves the room read #26341

andybalaam opened this issue Oct 9, 2023 · 8 comments · Fixed by matrix-org/matrix-react-sdk#11781 or matrix-org/matrix-react-sdk#12007
Labels
A-Element-R Issues affecting the port of Element's crypto layer to Rust A-Testing Testing, code coverage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist Z-Flaky-Test A test is raising false alarms Z-Labs

Comments

@andybalaam
Copy link
Member

Flaky test Read receipts > Paging up "After marking room as read, paging up to find old threads that were never read leaves the room read"
@andybalaam andybalaam added the Z-Flaky-Test A test is raising false alarms label Oct 9, 2023
@weeman1337 weeman1337 added S-Minor Impairs non-critical functionality or suitable workarounds exist O-Occasional Affects or can be seen by some users regularly or most users rarely labels Oct 9, 2023
@richvdh
Copy link
Member

richvdh commented Oct 12, 2023

https://github.com/matrix-org/matrix-js-sdk/actions/runs/6495998898/job/17642067954#step:8:1374

andybalaam added a commit to matrix-org/matrix-react-sdk that referenced this issue Oct 12, 2023
@andybalaam
Increase the timeout for the bot to enter the room
265f7bf 
in an attempt to fix flakes like:

* element-hq/element-web#26298
* element-hq/element-web#26341

which appear to fail when using Rust crypto, possibly because the client
takes a while to start up.
@andybalaam andybalaam mentioned this issue Oct 12, 2023
Merged
@andybalaam
Copy link
Member Author

https://github.com/matrix-org/matrix-js-sdk/actions/runs/6495998898/job/17642067954#step:8:1374

This looks like slow startup of the client. Hopefully fixed by matrix-org/matrix-react-sdk#11742

github-merge-queue bot pushed a commit to matrix-org/matrix-react-sdk that referenced this issue Oct 13, 2023
@andybalaam
Increase the timeout for the bot to enter the room (#11742)
6a99425 
in an attempt to fix flakes like:

* element-hq/element-web#26298
* element-hq/element-web#26341

which appear to fail when using Rust crypto, possibly because the client
takes a while to start up.
@richvdh
Copy link
Member

richvdh commented Oct 20, 2023

https://github.com/matrix-org/matrix-js-sdk/actions/runs/6590996937/job/17908708726

Looking at the screenshots, there are two different failure modes here.

  1. In one, it gets stuck at a loading spinner:

    image

The logs show it hanging at Init OlmMachine. This is ... odd. I don't know why that call would get stuck, nor do I know why only this test would get stuck there.

  1. The other failure mode is different:

    image

We don't seem to have useful logs, so I'm not really sure what happened here. In any case it seems likely to be yet another failure mode than the one observed

@kerryarchibald
Copy link
Contributor

https://github.com/matrix-org/matrix-js-sdk/actions/runs/6614847990/job/17965700609

@Johennes Johennes added the A-Testing Testing, code coverage, etc. label Oct 24, 2023
@florianduros florianduros mentioned this issue Oct 25, 2023
Merged
3 tasks
@richvdh
Copy link
Member

richvdh commented Oct 25, 2023

https://github.com/matrix-org/matrix-js-sdk/actions/runs/6636711970/job/18029638156#step:8:913

It's very odd that this seems to affect the rust-crypto build so much more than the legacy crypto build. I could believe some sort of systemic "rust is slow" thing going on, but why only this test out of the entire set of read receipts tests?

@florianduros florianduros added the A-Element-R Issues affecting the port of Element's crypto layer to Rust label Oct 26, 2023
@richvdh richvdh changed the title Flaky test Read receipts > Paging up "After marking room as read, paging up to find old threads that were never read leaves the room read" [Disabled] Flaky test Read receipts > Paging up "After marking room as read, paging up to find old threads that were never read leaves the room read" Oct 26, 2023
@richvdh richvdh changed the title [Disabled] Flaky test Read receipts > Paging up "After marking room as read, paging up to find old threads that were never read leaves the room read" [Disabled for Element-R] Flaky test Read receipts > Paging up "After marking room as read, paging up to find old threads that were never read leaves the room read" Oct 26, 2023
@richvdh richvdh changed the title [Disabled for Element-R] Flaky test Read receipts > Paging up "After marking room as read, paging up to find old threads that were never read leaves the room read" [Disabled for Element-R] Flaky test Read receipts > Paging up > After marking room as read, paging up to find old threads that were never read leaves the room read Oct 26, 2023
@richvdh richvdh changed the title [Disabled for Element-R] Flaky test Read receipts > Paging up > After marking room as read, paging up to find old threads that were never read leaves the room read [Disabled for Element-R] Flaky cypress test Read receipts > Paging up > After marking room as read, paging up to find old threads that were never read leaves the room read Oct 26, 2023
@andybalaam andybalaam mentioned this issue Nov 7, 2023
Merged
@andybalaam
Copy link
Member Author

Nope, didn't mean to close this. The skipIfRustCrypto is still there, and I'm not at all sure this is fixed. Will check.

@andybalaam andybalaam reopened this Nov 9, 2023
@andybalaam
Copy link
Member Author

I just ran this locally 10 times (with CYPRESS_RUST_CRYPTO=1) and it passed every time, so this is a candidate for trying out whether it's fixed.

@andybalaam
Copy link
Member Author

Leaving it for now though, because it uses Mark as Read, which we know has problems, that I am hoping to fix next.

@florianduros florianduros mentioned this issue Dec 6, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Element-R Issues affecting the port of Element's crypto layer to Rust A-Testing Testing, code coverage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist Z-Flaky-Test A test is raising false alarms Z-Labs
Projects
None yet
Milestone
No milestone
6 participants
@andybalaam @Johennes @richvdh @florianduros @kerryarchibald @weeman1337