It wouldn't be a bad idea to cover the changes from Bug 39294 with a Cypress test to prevent the same type of regression from happening again.
Created attachment 180802 [details] [review] Bug 39606: Add a cypress test To test: 1. APPLY PATCH 2. cypress run --spec "t/cypress/integration/Tools/" 3. Tests should pass
Sorry, the tests do not pass for me: $ cypress run --spec "t/cypress/integration/Tools/" It looks like this is your first time using Cypress: 12.17.4 ✔ Verified Cypress! /var/lib/koha/kohadev/.cache/Cypress/12.17.4/Cypress Opening Cypress... DevTools listening on ws://127.0.0.1:36659/devtools/browser/d019295e-472a-49f7-87c3-70639d72152f [10079:0411/113306.396390:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported. Missing baseUrl in compilerOptions. tsconfig-paths will be skipped ==================================================================================================== (Run Starting) ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ Cypress: 12.17.4 │ │ Browser: Electron 106 (headless) │ │ Node Version: v18.20.6 (/usr/bin/node) │ │ Specs: 1 found (ManageMarcImport_spec.ts) │ │ Searched: t/cypress/integration/Tools │ │ Experiments: experimentalStudio=true │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ ──────────────────────────────────────────────────────────────────────────────────────────────────── Running: ManageMarcImport_spec.ts (1 of 1) Breadcrumb tests ✓ Breadcrumbs (2236ms) loads the manage MARC import page ✓ loads the manage MARC import page (1164ms) 1) upload a MARC record 2 passing (27s) 1 failing 1) loads the manage MARC import page upload a MARC record: Timed out retrying after 10000ms + expected - actual -'replace' +'create_new' at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:91:34) (Results) ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ Tests: 3 │ │ Passing: 2 │ │ Failing: 1 │ │ Pending: 0 │ │ Skipped: 0 │ │ Screenshots: 4 │ │ Video: true │ │ Duration: 26 seconds │ │ Spec Ran: ManageMarcImport_spec.ts │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ (Screenshots) - /kohadevbox/koha/t/cypress/screenshots/ManageMarcImport_spec.ts/after_selection. (1000x1528) png - /kohadevbox/koha/t/cypress/screenshots/ManageMarcImport_spec.ts/right_before_sub (1000x1528) mission.png - /kohadevbox/koha/t/cypress/screenshots/ManageMarcImport_spec.ts/after_waiting.pn (1000x1216) g - /kohadevbox/koha/t/cypress/screenshots/ManageMarcImport_spec.ts/loads the manage (1280x720) MARC import page -- upload a MARC record (failed).png (Video) - Started compressing: Compressing to 32 CRF - Finished compressing: 1 second - Video output: /kohadevbox/koha/t/cypress/videos/ManageMarcImport_spec.ts.mp4 ==================================================================================================== (Run Finished) Spec Tests Passing Failing Pending Skipped ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ ✖ ManageMarcImport_spec.ts 00:26 3 2 1 - - │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ ✖ 1 of 1 failed (100%) 00:26 3 2 1 - -
Hey Magnus, thanks for testing! Can you confirm that you have Bug 39294 or do a git pull? It was only pushed yesterday to main, so maybe there is a chance you didn't have it?
All the tests are passing for me: Spec Tests Passing Failing Pending Skipped ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ ✔ ManageMarcImport_spec.ts 00:18 3 3 - - - │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ ✔ All specs passed! 00:18 3 3 - - -
Created attachment 180888 [details] [review] Bug 39606: Add a cypress test To test: 1. APPLY PATCH 2. cypress run --spec "t/cypress/integration/Tools/" 3. Tests should pass Signed-off-by: David Nind <david@davidnind.com>
Created attachment 181264 [details] [review] Bug 39606: Add a cypress test To test: 1. APPLY PATCH 2. cypress run --spec "t/cypress/integration/Tools/" 3. Tests should pass Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Great to see tests getting added :)
Pushed for 25.05! Well done everyone, thank you!
Why are you taking screenshots?
The wait statements will certainly cause random failures. cy.wait(5000); cy.wait(2000);
Created attachment 181355 [details] [review] Bug 39606: Remove waits and screenshots
Notice the following couple of lines: cy.get("#fileuploadstatus").contains("100%"); cy.get("legend").contains("Look for existing records in catalog?").should('be.visible'); It makes Cypress wait 10s until "100%" appears (ie. the file is updated), then the block with the dropdown lists you are going to interact with to be visible. You should almost never use cy.wait.
(In reply to Jonathan Druart from comment #12) > Notice the following couple of lines: > cy.get("#fileuploadstatus").contains("100%"); > cy.get("legend").contains("Look for existing records in > catalog?").should('be.visible'); > > It makes Cypress wait 10s until "100%" appears (ie. the file is updated), > then the block with the dropdown lists you are going to interact with to be > visible. > > You should almost never use cy.wait. Can you please respond to this?
(In reply to Katrin Fischer from comment #13) > (In reply to Jonathan Druart from comment #12) > > Notice the following couple of lines: > > cy.get("#fileuploadstatus").contains("100%"); > > cy.get("legend").contains("Look for existing records in > > catalog?").should('be.visible'); > > > > It makes Cypress wait 10s until "100%" appears (ie. the file is updated), > > then the block with the dropdown lists you are going to interact with to be > > visible. > > > > You should almost never use cy.wait. > > Can you please respond to this? Unless I am mistaken Jonathan's follow-up dealt with this. Is there more to do here?
Yes, all good here.
Last patch has not been pushed :-/
(In reply to Jonathan Druart from comment #11) > Created attachment 181355 [details] [review] [review] > Bug 39606: Remove waits and screenshots Moved to bug 40046.