Skip to content

Conversation

@szaimen
Copy link
Contributor

@szaimen szaimen commented Jan 3, 2023

Disclaimer: I have not tested this.

Possibly closes nextcloud/server#35955

@szaimen szaimen added the 2. developing Work in progress label Jan 3, 2023
@szaimen szaimen force-pushed the enh/noid/update-vue-to-7.3 branch from af20b0a to 6e632d0 Compare January 3, 2023 11:38
@szaimen
Copy link
Contributor Author

szaimen commented Jan 3, 2023

/compile amend /

@szaimen szaimen added 3. to review Waiting for reviews dependencies Pull requests that update a dependency file and removed 2. developing Work in progress labels Jan 3, 2023
@szaimen szaimen added this to the Nextcloud 25.0.3 milestone Jan 3, 2023
@szaimen szaimen marked this pull request as ready for review January 3, 2023 11:39
Signed-off-by: Simon L <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the enh/noid/update-vue-to-7.3 branch from 6e632d0 to 3f04426 Compare January 3, 2023 11:41
@szaimen szaimen changed the title update vue to 7.3 [stable25] update vue to 7.3 Jan 3, 2023
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works.

@szaimen
Copy link
Contributor Author

szaimen commented Jan 3, 2023

@skjnldsv any idea why the cypress test fails?

@nickvergessen
Copy link
Member

  Visual regression tests 
    ✓ See files in the list (11797ms)
    ✓ Open the viewer on file click (711ms)
    ✓ See the menu icon and title on the viewer header (180ms)
    ✓ Does see next navigation arrows (98ms)
    ✓ Does not see a loading animation (37ms)
    ✓ Take test-card.mp4 screenshot (1653ms)
    ✓ Show second file on next (367ms)
    ✓ Does not see a loading animation (70ms)
    (Attempt 1 of 3) Take test-card.png screenshot
    (Attempt 2 of 3) Take test-card.png screenshot
    1) Take test-card.png screenshot
    ✓ Close and open image again (1437ms)
    ✓ Does not see a loading animation (58ms)
    (Attempt 1 of 3) Take test-card.png screenshot 2
    (Attempt 2 of 3) Take test-card.png screenshot 2
    2) Take test-card.png screenshot 2
    ✓ Open non-dav image (213ms)
    ✓ Does not see a loading animation (45ms)
    (Attempt 1 of 3) Take non-dav logo.png screenshot
    (Attempt 2 of 3) Take non-dav logo.png screenshot
    3) Take non-dav logo.png screenshot


  12 passing (36s)
  3 failing

  1) Visual regression tests 
       Take test-card.png screenshot:
     Error: The "image" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.013541666666666665
      at compareSnapshotsPlugin (/home/runner/work/viewer/viewer/node_modules/cypress-visual-regression/dist/plugin.js:81:13)
      at runMicrotasks (<anonymous>:null:null)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  2) Visual regression tests 
       Take test-card.png screenshot 2:
     Error: The "image2" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.013541666666666665
      at compareSnapshotsPlugin (/home/runner/work/viewer/viewer/node_modules/cypress-visual-regression/dist/plugin.js:81:13)
      at runMicrotasks (<anonymous>:null:null)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  3) Visual regression tests 
       Take non-dav logo.png screenshot:
     Error: The "non-dav" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.012586506222494345
      at compareSnapshotsPlugin (/home/runner/work/viewer/viewer/node_modules/cypress-visual-regression/dist/plugin.js:81:13)
      at runMicrotasks (<anonymous>:null:null)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

So maybe
https://github.com/nextcloud/viewer/blob/master/package.json#L38

@st3iny
Copy link
Member

st3iny commented Jan 3, 2023

There are visual regressions. The snapshots have to be compared and the snapshots have to be updated if the breakage is minor.

Take a look at:

  1) Visual regression tests 
       Take test-card.png screenshot:
     Error: The "image" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.013541666666666665
      at compareSnapshotsPlugin (/home/runner/work/viewer/viewer/node_modules/cypress-visual-regression/dist/plugin.js:81:13)
      at runMicrotasks (<anonymous>:null:null)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  2) Visual regression tests 
       Take test-card.png screenshot 2:
     Error: The "image2" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.013541666666666665
      at compareSnapshotsPlugin (/home/runner/work/viewer/viewer/node_modules/cypress-visual-regression/dist/plugin.js:81:13)
      at runMicrotasks (<anonymous>:null:null)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  3) Visual regression tests 
       Take non-dav logo.png screenshot:
     Error: The "non-dav" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.012586506222494345
      at compareSnapshotsPlugin (/home/runner/work/viewer/viewer/node_modules/cypress-visual-regression/dist/plugin.js:81:13)
      at runMicrotasks (<anonymous>:null:null)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

@szaimen
Copy link
Contributor Author

szaimen commented Jan 3, 2023

So maybe
https://github.com/nextcloud/viewer/blob/master/package.json#L38

the command does not work on my system:

npm run cypress:update-snapshots

> [email protected] cypress:update-snapshots
> TESTING=true cypress run --env type=base --spec cypress/e2e/visual-regression.cy.js --config screenshotsFolder=cypress/snapshots/base

It looks like this is your first time using Cypress: 10.11.0

✔  Verified Cypress! /root/.cache/Cypress/10.11.0/Cypress

Opening Cypress...
[11362:0103/125333.844814:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported.
Missing baseUrl in compilerOptions. tsconfig-paths will be skipped
Your configFile is invalid: /root/viewer/cypress.config.ts

It threw an error when required, check the stack trace below:

/root/viewer/cypress/dockerNode.ts:142
            ip = data?.NetworkSettings?.IPAddress || '';
                      ^

SyntaxError: Unexpected token '.'
    at wrapSafe (internal/modules/cjs/loader.js:915:16)
    at Module._compile (internal/modules/cjs/loader.js:963:27)
    at Module.m._compile (/root/.cache/Cypress/10.11.0/Cypress/resources/app/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Object.require.extensions.<computed> [as .ts] (/root/.cache/Cypress/10.11.0/Cypress/resources/app/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/root/viewer/cypress.config.ts:2:1)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Module.m._compile (/root/.cache/Cypress/10.11.0/Cypress/resources/app/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Object.require.extensions.<computed> [as .ts] (/root/.cache/Cypress/10.11.0/Cypress/resources/app/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)

@szaimen
Copy link
Contributor Author

szaimen commented Jan 3, 2023

@skjnldsv could you please lent me a hand here? I don't know how to proceed with this...

@skjnldsv
Copy link
Member

skjnldsv commented Jan 3, 2023

Signed-off-by: Simon L <[email protected]>
@szaimen
Copy link
Contributor Author

szaimen commented Jan 3, 2023

@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 3, 2023
@szaimen szaimen enabled auto-merge January 3, 2023 13:23
@szaimen szaimen merged commit 3aa50e0 into stable25 Jan 3, 2023
@szaimen szaimen deleted the enh/noid/update-vue-to-7.3 branch January 3, 2023 13:29
@nickvergessen
Copy link
Member

Judging from the screenshot this here does not match anymore:
https://github.com/nextcloud/viewer/blob/master/src/views/Viewer.vue#L974

Might be worth creating a follow up

@szaimen
Copy link
Contributor Author

szaimen commented Jan 3, 2023

Judging from the screenshot this here does not match anymore: https://github.com/nextcloud/viewer/blob/master/src/views/Viewer.vue#L974

Might be worth creating a follow up

I don't understand... the modal header was simply updated which is contained in vue IIRC...

@nickvergessen
Copy link
Member

And in Viewer.vue it was overwritten to be white instead of "max contrast" but a parent selector changed so now the rule does not match anymore. But not my beer :P

@skjnldsv skjnldsv mentioned this pull request Jan 6, 2023
5 tasks
@blizzz blizzz mentioned this pull request Jan 11, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants