Skip to content

test(playwright): migrate propfind tests from Cypress to Playwright - #9021

Merged
mejo- merged 4 commits into
mainfrom
test/propfind_playwright
Aug 13, 2026
Merged

test(playwright): migrate propfind tests from Cypress to Playwright#9021
mejo- merged 4 commits into
mainfrom
test/propfind_playwright

Conversation

@mejo-

@mejo- mejo- commented Aug 9, 2026

Copy link
Copy Markdown
Member

Assisted-by: OpenCode:claude-fable-5

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests

🤖 AI (if applicable)

  • The content of this PR was partly generated using AI tools
  • The AI-generated content was reviewed, comprehended and tested by a human

@mejo- mejo- self-assigned this Aug 9, 2026
@mejo- mejo- added the tests If you write them we ♥ you label Aug 9, 2026
@github-project-automation github-project-automation Bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity team Aug 9, 2026
@mejo-

mejo- commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

/backport to stable34

@mejo-

mejo- commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

/backport to stable33

Signed-off-by: Jonas <jonas@freesources.org>
Assisted-by: OpenCode:claude-fable-5
@mejo-
mejo- force-pushed the test/propfind_playwright branch from 0282101 to b9c8505 Compare August 10, 2026 07:35
Comment thread playwright/e2e/propfind.spec.ts Outdated
const [root1] = await propfindFolder(user, '/', 0, properties)
expect(root1).toHaveProperty(PROPERTY_WORKSPACE_FLAT, '')

await deleteWebDAVResource(user, '/Readme.md')

@mejo- mejo- Aug 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This line most likely fixes the flakiness of the test compared to the Cypress one. Without this line, this test succeeds reliably locally but fails reliably (so far) in CI.

Claude assumes that this is because the storage cache doesn't get invalidated properly in CI when only overwriting the file compared to deleting it and uploading a new file (which results in new fileId etc). It may well be that the missing cache invalidation is due to different APCu caching setup in CI.

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.

I wonder if we could fix that different setup. Would such a setup have visible consequences in actual setups or is this a CI artifact we are observing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't have hard feelings here. I didn't dive into investigating the actual differences with caching setup between CI and local environment, I merely was happy to have found an explanation for the flaky test and decided to move on 😆

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@max-nextcloud do you see a problem with adding the delete in the test? It arguably changes what is being tested: updating a file no longer is tested. But do you think it should be tested in the propfind tests?

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.

Well... this started out as a regression test. Would be interesting to see if this actually represents a user visible bug. So if one user creates an empty readme.md and then another one adds content... does the content show? I'll try.

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.

So I tried this on cloud.nextcloud.com and it works as intended. However we may not have the problematic caching config and I think I remember that the propfind was also used by the desktop client.
In the browser it basically directly opens an editing session - editable or read only and dynamically syncs the content.

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.

Yes... this initially was about android: nextcloud/server#32548 (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.

I was hoping the backports would tell us if this is present in previous versions as well. But they do not work automatically.

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.

test is passing in stable34 so this seems to be a recent regression.

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.

The test was just being flaky. The underlying cause is that Etags will not be updated if the mtime does not change. So if the file was initially written in the same second the update happened the update will not be propagated.

This problem is specific to our test setup - as usually one does not update a file multiple times per second. I'll adjust the test to account for this.

@mejo- mejo- moved this from 🧭 Planning evaluation (don't pick) to 👀 In review in 📝 Productivity team Aug 10, 2026
mejo- and others added 2 commits August 11, 2026 14:58
…slow

The test often exceeds the default timeout in CI.

Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud

Copy link
Copy Markdown
Collaborator

/backport! to stable34 please

@max-nextcloud

Copy link
Copy Markdown
Collaborator

/backport! to stable33 please

@backportbot

backportbot Bot commented Aug 11, 2026

Copy link
Copy Markdown

The backport to stable33 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable33
git pull origin stable33

# Create the new backport branch
git checkout -b backport/9021/stable33

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick b9c85053 86f6d68a bd5cc743

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/9021/stable33

Error: Failed to check for changes with origin/stable33: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

Etags will not be updated if the mtime does not change.
So if the file was overwritten within the same second it was created
the mtime and the etag will remain the same
and the outdated cached content will be distributed.

This problem is specific to our test setup.
Usually one does not update a file multiple times per second.
Adjust the test to ensure the mtime changes.

Signed-off-by: Max <max@nextcloud.com>
@mejo-
mejo- merged commit 4c9eb08 into main Aug 13, 2026
80 of 89 checks passed
@mejo-
mejo- deleted the test/propfind_playwright branch August 13, 2026 09:53
@github-project-automation github-project-automation Bot moved this from 👀 In review to ☑️ Done in 📝 Productivity team Aug 13, 2026
@backportbot

backportbot Bot commented Aug 13, 2026

Copy link
Copy Markdown

The backport to stable33 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable33
git pull origin stable33

# Create the new backport branch
git checkout -b backport/9021/stable33

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick b9c85053 86f6d68a bd5cc743 cb19d998

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/9021/stable33

Error: Failed to check for changes with origin/stable33: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot

backportbot Bot commented Aug 13, 2026

Copy link
Copy Markdown

The backport to stable34 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable34
git pull origin stable34

# Create the new backport branch
git checkout -b backport/9021/stable34

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick b9c85053 86f6d68a bd5cc743 cb19d998

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/9021/stable34

Error: Failed to create pull request: Validation Failed: {"resource":"PullRequest","code":"custom","message":"A pull request already exists for nextcloud:backport/9021/stable34."} - https://docs.github.com/rest/pulls/pulls#create-a-pull-request


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@mejo-

mejo- commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

/backport to stable34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review tests If you write them we ♥ you

Projects

Status: ☑️ Done

Development

Successfully merging this pull request may close these issues.

2 participants