Bug 39606 - Cover change from bug 39294 with a Cypress test
Summary: Cover change from bug 39294 with a Cypress test
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass (lukeg)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 39294
Blocks: 40046
  Show dependency treegraph
 
Reported: 2025-04-10 22:06 UTC by Lucas Gass (lukeg)
Modified: 2025-06-02 10:14 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This adds Cyress tests for staging MARC records for import.
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39606: Add a cypress test (11.21 KB, patch)
2025-04-10 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 39606: Add a cypress test (11.26 KB, patch)
2025-04-12 03:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 39606: Add a cypress test (11.32 KB, patch)
2025-04-22 13:56 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39606: Remove waits and screenshots (2.97 KB, patch)
2025-04-23 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2025-04-10 22:06:19 UTC
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.
Comment 1 Lucas Gass (lukeg) 2025-04-10 22:09:21 UTC
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
Comment 2 Magnus Enger 2025-04-11 11:35:20 UTC
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        -        -
Comment 3 Lucas Gass (lukeg) 2025-04-11 16:43:55 UTC
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?
Comment 4 David Nind 2025-04-12 03:13:24 UTC
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        -        -        -
Comment 5 David Nind 2025-04-12 03:14:08 UTC
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>
Comment 6 Martin Renvoize (ashimema) 2025-04-22 13:56:51 UTC
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>
Comment 7 Martin Renvoize (ashimema) 2025-04-22 13:57:10 UTC
Great to see tests getting added :)
Comment 8 Katrin Fischer 2025-04-23 07:43:20 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 9 Jonathan Druart 2025-04-23 09:11:21 UTC
Why are you taking screenshots?
Comment 10 Jonathan Druart 2025-04-23 09:12:01 UTC
The wait statements will certainly cause random failures.

cy.wait(5000);
cy.wait(2000);
Comment 11 Jonathan Druart 2025-04-23 09:49:01 UTC
Created attachment 181355 [details] [review]
Bug 39606: Remove waits and screenshots
Comment 12 Jonathan Druart 2025-04-23 09:49:19 UTC
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.
Comment 13 Katrin Fischer 2025-05-16 15:29:34 UTC
(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?
Comment 14 Lucas Gass (lukeg) 2025-05-16 15:49:32 UTC
(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?
Comment 15 Jonathan Druart 2025-05-19 07:58:35 UTC
Yes, all good here.
Comment 16 Jonathan Druart 2025-06-02 10:13:36 UTC
Last patch has not been pushed :-/
Comment 17 Jonathan Druart 2025-06-02 10:14:51 UTC
(In reply to Jonathan Druart from comment #11)
> Created attachment 181355 [details] [review] [review]
> Bug 39606: Remove waits and screenshots

Moved to bug 40046.