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

fix(cypress): close attachments file before proceeding #4734

Merged
merged 2 commits into from Aug 29, 2023

Conversation

max-nextcloud
Copy link
Collaborator

📝 Summary

Deleting the file later on failed because the file still was locked.

🖼️ Screenshots

🏚️ Before 🏡 After
Test all attachment insertion methods -- test if attachment folder is deleted after having deleted a markdown file (failed)
A

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • this PR fixes tests.
  • Documentation is not required

@max-nextcloud
Copy link
Collaborator Author

@juliushaertl frankly i have no idea why the delete request would fail with a 423 for a file that has just been copied.

Do you know why it might fail?

@cypress
Copy link

cypress bot commented Aug 28, 2023

2 flaky tests on run #11913 ↗︎

0 133 18 0 Flakiness 2

Details:

fix(cypress): close attachments file before proceeding
Project: Text Commit: 4f148f95e8
Status: Passed Duration: 03:47 💡
Started: Aug 29, 2023 6:47 PM Ended: Aug 29, 2023 6:51 PM
Flakiness  attachments.spec.js • 1 flaky test

View Output Video

Test Artifacts
Test all attachment insertion methods > See test files in the list and display hidden files Output Screenshots
Flakiness  nodes/FrontMatter.spec.js • 1 flaky test

View Output Video

Test Artifacts
Front matter support > Reopen front matter Output Screenshots

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

@max-nextcloud max-nextcloud force-pushed the fix/cy-attachments-close-file branch 2 times, most recently from 3288448 to e5fa2a9 Compare August 28, 2023 05:20
@max-nextcloud
Copy link
Collaborator Author

@juliushaertl
Copy link
Member

juliushaertl commented Aug 29, 2023

frankly i have no idea why the delete request would fail with a 423 for a file that has just been copied.

This seems still one flakyness where there is something wrong with the server session handling in case the preview service worker doesn't finish a request and therefore the browser holds outdated csrf token information. I'd need to dive further into that again, but seemed less regular than back then with #4350.

A retry seemed to have worked, but the one seems to fail still

Test all attachment insertion methods > test if attachment folder is deleted after having deleted a markdown file

@juliushaertl
Copy link
Member

The file shouldn't be locked on CI as files_lock is not enabled there

@max-nextcloud
Copy link
Collaborator Author

Formatted nextcloud.log - only contains a theming error

This error shows up twice. Once for text.svg and once for folder.svg.

{
  "reqId": "cvt0qoFQID5kFwXhcgoS",
  "level": 3,
  "time": "2023-08-28T05:45:23+00:00",
  "remoteAddr": "127.0.0.1",
  "user": "ongosb",
  "app": "index",
  "method": "GET",
  "url": "/index.php/apps/theming/img/core/filetypes/folder.svg?v=47d7f13e",
  "message": "Could not create folder \"/appdata_ocl4l7eqltkx/theming/global\"",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Cypress/12.17.4 Chrome/106.0.5249.51 Electron/21.0.0 Safari/537.36",
  "version": "28.0.0.2",
  "exception": {
    "Exception": "OCP\\Files\\NotPermittedException",
    "Message": "Could not create folder \"/appdata_ocl4l7eqltkx/theming/global\"",
    "Code": 0,
    "Trace": [
      {
        "file": "/home/runner/work/text/text/lib/private/Files/AppData/AppData.php",
        "line": 147,
        "function": "newFolder",
        "class": "OC\\Files\\Node\\Folder",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/apps/theming/lib/ImageManager.php",
        "line": 383,
        "function": "newFolder",
        "class": "OC\\Files\\AppData\\AppData",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/apps/theming/lib/ImageManager.php",
        "line": 173,
        "function": "getRootFolder",
        "class": "OCA\\Theming\\ImageManager",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/apps/theming/lib/ImageManager.php",
        "line": 190,
        "function": "getCacheFolder",
        "class": "OCA\\Theming\\ImageManager",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/apps/theming/lib/Controller/IconController.php",
        "line": 97,
        "function": "getCachedImage",
        "class": "OCA\\Theming\\ImageManager",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/lib/private/AppFramework/Http/Dispatcher.php",
        "line": 230,
        "function": "getThemedIcon",
        "class": "OCA\\Theming\\Controller\\IconController",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/lib/private/AppFramework/Http/Dispatcher.php",
        "line": 137,
        "function": "executeController",
        "class": "OC\\AppFramework\\Http\\Dispatcher",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/lib/private/AppFramework/App.php",
        "line": 184,
        "function": "dispatch",
        "class": "OC\\AppFramework\\Http\\Dispatcher",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/lib/private/Route/Router.php",
        "line": 315,
        "function": "main",
        "class": "OC\\AppFramework\\App",
        "type": "::"
      },
      {
        "file": "/home/runner/work/text/text/lib/base.php",
        "line": 1070,
        "function": "match",
        "class": "OC\\Route\\Router",
        "type": "->"
      },
      {
        "file": "/home/runner/work/text/text/index.php",
        "line": 37,
        "function": "handleRequest",
        "class": "OC",
        "type": "::"
      }
    ],
    "File": "/home/runner/work/text/text/lib/private/Files/Node/Folder.php",
    "Line": 162,
    "message": "Could not create folder \"/appdata_ocl4l7eqltkx/theming/global\"",
    "exception": {},
    "CustomMessage": "Could not create folder \"/appdata_ocl4l7eqltkx/theming/global\""
  }
}

Deleting the file later on failed because the file still was locked.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
Copy link
Member

Seemed that the retries were hiding the original failure since attachment tests are not well isolated
Screenshot 2023-08-29 at 20 42 00

@juliushaertl juliushaertl added bug Something isn't working tests If you write them we ♥ you 2. developing labels Aug 29, 2023
@juliushaertl juliushaertl merged commit 3dc9645 into main Aug 29, 2023
30 checks passed
@juliushaertl juliushaertl deleted the fix/cy-attachments-close-file branch August 29, 2023 18:57
@juliushaertl
Copy link
Member

Thanks for starting this @max-nextcloud Seems we have a somewhat green CI again 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing bug Something isn't working tests If you write them we ♥ you
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants