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.

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

Investigate failing sync service test on stable27 #5154

Closed
max-nextcloud opened this issue Dec 21, 2023 · 5 comments · Fixed by #5604
Closed

Investigate failing sync service test on stable27 #5154

max-nextcloud opened this issue Dec 21, 2023 · 5 comments · Fixed by #5604
Labels
bug: regression bug Something isn't working technical debt tests If you write them we ♥ you

Comments

@max-nextcloud
Copy link
Collaborator

Describe the bug
For a while the sync.spec.js has been failing on the stable27 branch:

grafik

Looks like it may have been introduced by #5087 or at the same time. The cypress run in the PR passed though.

To Reproduce
Steps to reproduce the behavior:

  1. Go to cypress action run overview for stable27 branch
  2. See the failed runs
  3. Scroll down till you see green runs again.
  4. Inspect individual failed runs to see most of them fail in sync.spec.js.

Expected behavior
Cypress should pass on all integration branches.

@max-nextcloud max-nextcloud added the bug Something isn't working label Dec 21, 2023
@max-nextcloud
Copy link
Collaborator Author

grafik

Sync -- saves the actual file and document state (failed)

Server log is probably unrelated - maybe from a retry:

{
  "reqId": "XNUpWmg1ATmxO2rATg7u",
  "level": 3,
  "time": "2023-12-21T05:51:19+00:00",
  "remoteAddr": "127.0.0.1",
  "user": "admin",
  "app": "ocs_api",
  "method": "POST",
  "url": "/ocs/v2.php/cloud/users?format=json",
  "message": "Failed addUser attempt: User already exists.",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Cypress/13.6.1 Chrome/114.0.5735.289 Electron/25.8.4 Safari/537.36",
  "version": "27.1.5.1",
  "data": {
    "app": "ocs_api"
  }
}

@max-nextcloud max-nextcloud added tests If you write them we ♥ you bug: regression labels Dec 21, 2023
@max-nextcloud max-nextcloud changed the title Investigate failing sync service test Investigate failing sync service test on stable27 Dec 21, 2023
@max-nextcloud
Copy link
Collaborator Author

running the last passing commit of text with the latest commit of server stable27 passes: https://github.com/nextcloud/text/actions/runs/7104754058/job/19905039693
At the same time the current state of text with the last passing commit of server still fails: https://github.com/nextcloud/text/actions/runs/7303886691/job/19905235351

So looks like this somehow broke on the text side of things rather than the server side.

@mejo-
Copy link
Member

mejo- commented Apr 3, 2024

I tried to understand what happens here, but failed so far.

Two tests in sync.spec.js fail reliably when running them with cypress run locally - even though they pass reliably with cypress open:

  • saves the actual file and document state
  • saves on close

Both tests load a document, edit the document, download the file and see whether the edits are in the downloaded file. The first one does a manual save before download, the second closes the document before download.

In the screenshots it's obvious that the file got saved. I suspected a race condition, but adding cy.wait() at different places in the test doesn't change the situation. The downloaded file always stays the one initially uploaded, without the editing changes.

@mejo-
Copy link
Member

mejo- commented Apr 3, 2024

Ok, found a reliable fix now: #5601

@mejo-
Copy link
Member

mejo- commented Apr 3, 2024

Fixed by #5604

@mejo- mejo- closed this as completed Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: regression bug Something isn't working technical debt tests If you write them we ♥ you
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants