Skip to content

tests(cypress): Refactor reconnect test to be more reliable and add second test for actual reconnect - #4507

Merged
juliusknorr merged 1 commit into
mainfrom
cypress/reconnect-test
Jul 13, 2023
Merged

tests(cypress): Refactor reconnect test to be more reliable and add second test for actual reconnect#4507
juliusknorr merged 1 commit into
mainfrom
cypress/reconnect-test

Conversation

@juliusknorr

@juliusknorrjuliusknorr commented Jul 12, 2023

Copy link
Copy Markdown
Member

This should make our top failure cypress test on main more reliable and also extends it with another case:

  • The existing test only tests if an existing session is picket up after a short interrupt of sync/push requests
  • The new test actually waits until the y.js websocket connection gets closed and checks if a full reconnect is working

@juliusknorrjuliusknorr added tests If you write them we ♥ you 3. to review labels Jul 12, 2023
@juliusknorrjuliusknorr added this to the Nextcloud 28 milestone Jul 12, 2023
@cypress

cypressBot commented Jul 12, 2023

Copy link
Copy Markdown

2 flaky tests on run #11073 ↗︎

014920Flakiness2

Details:

tests(cypress): Refactor reconnect test to be more reliable and add second test ...
Project: TextCommit:712f54a440
Status: PassedDuration: 04:29 💡
Started: Jul 13, 2023 7:49 AMEnded: Jul 13, 2023 7:54 AM
Flakiness nodes/HardBreak.spec.js • 1 flaky test

ViewOutputVideo

Test Artifacts
Hard break support > Do not create hard breaks within headings OutputScreenshots
Flakiness api/UsersApi.spec.js • 1 flaky test

ViewOutputVideo

Test Artifacts
The user mention API > fetches users with valid session OutputScreenshots

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@juliusknorr
juliusknorrforce-pushed the cypress/reconnect-test branch from 70339f9 to 652080aCompareJuly 12, 2023 22:31

@mejo-mejo- left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome, thanks for tackling this ✨

@max-nextcloudmax-nextcloud left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yay! Thanks for getting to the bottom of this! 🤿
Also great idea to test the close / create loop.

I have just one small remark.

Comment threadcypress/e2e/sync.spec.js Outdated
if (reconnect) {
req.continue()
req.alias = 'alive'
reconnect = true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems somewhat redundant as reconnect is already checked to be truethy in line 70

Or am I missing something?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Right, seems I've left this from the other test where the toggle re-enable connections is triggered by the close/connect

@max-nextcloud

max-nextcloud commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

Maybe also backport to stable27...
grafik

and stable26
grafik

@max-nextcloud

Copy link
Copy Markdown
Collaborator

/backport to stable27

@max-nextcloud

Copy link
Copy Markdown
Collaborator

/backport to stable26

…econd test for actual reconnect
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewtestsIf you write them we ♥ you

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@juliusknorr@max-nextcloud@mejo-