Bug 40174

Summary: Add a way to cleanly insert data in DB from Cypress tests
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: Passed QA --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: david, dcook, martin.renvoize, matt.blenkinsop
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 40169, 40170, 40173, 40179, 40344    
Bug Blocks: 40180, 40346    
Attachments: Bug 40174: Allow proper E2E testing with Cypress
Bug 40174: Introduce Cypress auth plugin
Bug 40174: Use api-client
Bug 40174 (and related bugs) - Test failues.txt
Bug 40174: Deal with enum
Bug 40174: Set minLength to 3
Bug 40174: Set minLength to 3
Bug 40174: Require RESTBasicAuth turned ON
Bug 40174: Allow proper E2E testing with Cypress
Bug 40174: Introduce Cypress auth plugin
Bug 40174: Use api-client
Bug 40174: Deal with enum
Bug 40174: Set minLength to 3
Bug 40174: Require RESTBasicAuth turned ON
Bug 40174: Do not generate bigint
Bug 40174: Allow proper E2E testing with Cypress
Bug 40174: Introduce Cypress auth plugin
Bug 40174: Use api-client
Bug 40174: Deal with enum
Bug 40174: Set minLength to 3
Bug 40174: Require RESTBasicAuth turned ON
Bug 40174: Do not generate bigint

Description Jonathan Druart 2025-06-18 21:10:48 UTC
We need a way to insert into the DB the objects generated via mockData
Comment 1 Jonathan Druart 2025-06-19 10:50:58 UTC
Created attachment 183372 [details] [review]
Bug 40174: Allow proper E2E testing with Cypress

This is the first step toward implementing true end-to-end testing with
Cypress.

Until now, we have been mocking responses using cy.intercept(), but this
approach can lead to confusion. In some cases, we have even had to mock
global JavaScript variables, which makes the code unnecessarily complex
and unconventional (e.g. win.categories_map).

All the bug reports listed under this tree have helped lay the
groundwork for this patch. With it, we are able to build mock objects
(via plugin mockData) and insert them directly into the database
(insertData). Once the tests are complete, we’ll restore the database to
its previous state by removing any data generated during the tests.

The tests in KohaTable/Holdings_spec.ts have been updated to leverage
this new setup.

Additionally, a caching mechanism has been added to prevent _id
attributes from being generated with the same values (a rare issue we
occasionally encountered).

While this is still not a perfect solution, it introduces a solid
foundation for future improvements.

Test plan:
All Cypress tests must pass
Comment 2 Jonathan Druart 2025-06-19 10:50:59 UTC
Created attachment 183373 [details] [review]
Bug 40174: Introduce Cypress auth plugin

So we can easily retrieve the value for the Authorization header.
Comment 3 Jonathan Druart 2025-06-19 10:51:01 UTC
Created attachment 183374 [details] [review]
Bug 40174: Use api-client
Comment 4 David Nind 2025-07-03 10:01:18 UTC
I attempted to test, but one of the dependent bugs doesn't apply:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 40173: Make http-client reusable from Cypress tests
Applying: Bug 40173: Return the response early if the caller needs it
Applying: Bug 40173: Add test for the api-client Cypress plugin
Using index info to reconstruct a base tree...
M	t/cypress/plugins/index.js
Falling back to patching base and 3-way merge...
Auto-merging t/cypress/plugins/index.js
CONFLICT (content): Merge conflict in t/cypress/plugins/index.js
error: Failed to merge in the changes.
Patch failed at 0001 Bug 40173: Add test for the api-client Cypress plugin

However, it applies fine if I just apply it by itself.
Comment 5 Jonathan Druart 2025-07-03 10:36:19 UTC
Yes the tree is a bit complicated. If you want to have everything applied you can have a look at the branch `bug_40180` on my gitlab repo.
Comment 6 David Nind 2025-07-07 23:47:28 UTC
Created attachment 183851 [details]
Bug 40174 (and related bugs) - Test failues.txt

I tested the bug_40180 branch from your repository.

This is how I tested things:
1. Added the remote: git remote add joubu https://gitlab.com/joubu/Koha
2. Fetched the remote: git fetch joubu
3. Checked out the branch from the remote: git checkout -b bug_40180 joubu/bug_40180
4. yarn build
5. restart_all (maybe this should be a reset_all?)
6. Enabled the RESTBasicAuth system preference.
7. Ran individual tests for the dependent and related bugs - see [1] for the list
8. Ran all the cypress tests: perl /kohadevbox/misc4dev/run_tests.pl --run-cypress-tests-only
   - ERM/DataProviders_spec.ts and ERM/UsageReports_spec.ts failed, but I think they've been fixed now
   - KohaTable/Holdings_spec.ts sometimes failed for me [2]
   - KohaTable/KohaTable_spec.ts sometimes failed for me [3]
9. The tests related to bugs 40301 and 40320 failed for me.

[1] All the interrelated bugs
=============================

40168 - Signed off
- yarn cypress run --spec t/cypress/integration/KohaTable/Holdings_spec.ts

40169 - Signed off
- yarn cypress run --spec t/cypress/integration/t/mockData.ts

40170
- yarn cypress run --spec t/cypress/integration/Auth/csrf.ts
- yarn cypress run --spec t/cypress/integration/Islands/AcquisitionsMenu_spec.ts
- yarn cypress run --spec t/cypress/integration/KohaTable/Holdings_spec.ts
- yarn cypress run --spec t/cypress/integration/KohaTable/PatronSearch_spec.ts
- yarn cypress run --spec t/cypress/integration/t/db.ts

40172 (Signed off)
- No additional tests to run

40173
- yarn cypress run --spec t/cypress/integration/t/api-client.ts

40174
- yarn cypress run --spec t/cypress/integration/KohaTable/Holdings_spec.ts  (failed??? passed when run before on 40168)
- yarn cypress run --spec t/cypress/integration/t/insertData.ts

40175 - OMNIBUS bug (no actual patches)

40176 (Passed QA)
- prove xt/api.t 

40177 (Passed QA)
- prove xt/api.t 

40178 (Passed QA)
- provec xt/api.t 

40179 (Passed QA)
- prove xt/api.t

40180
- yarn cypress run --spec t/cypress/integration/KohaTable/Holdings_spec.ts
- yarn cypress run --spec t/cypress/integration/KohaTable/PendingHolds_spec.ts
- yarn cypress run --spec t/cypress/integration/t/insertData.ts

40181 - Applied on top
- yarn cypress run --spec t/cypress/integration/t/insertData.ts

40301 - Applied on top as well as 40281: both tests failed with and without 40281
- yarn cypress run --spec t/cypress/integration/t/insertData.ts
- yarn cypress run --spec t/cypress/integration/KohaTable/CirculationHistory_spec.ts

40319 - Applied on top: OK
- Follow the test plan in the bug (no tests to run yet)

40320 - Applied on top: tests fail for me [5]
- Follow the test plan
- Make sure the display of city, state and zipcode is the same as before the patch
- yarn cypress run --spec t/cypress/integration/Patron/Address.ts

[2], [3], [4], [5] - see attachment to the bug
===============================================
Comment 7 Jonathan Druart 2025-07-08 09:18:13 UTC
(In reply to David Nind from comment #6)
> Created attachment 183851 [details]
> Bug 40174 (and related bugs) - Test failues.txt
> 
> I tested the bug_40180 branch from your repository.
> 
> This is how I tested things:
> 1. Added the remote: git remote add joubu https://gitlab.com/joubu/Koha
> 2. Fetched the remote: git fetch joubu
> 3. Checked out the branch from the remote: git checkout -b bug_40180
> joubu/bug_40180
> 4. yarn build
> 5. restart_all (maybe this should be a reset_all?)

Yes, `reset_all` or `yarn api:bundle && restart_all`

> 6. Enabled the RESTBasicAuth system preference.
> 7. Ran individual tests for the dependent and related bugs - see [1] for the
> list
> 8. Ran all the cypress tests: perl /kohadevbox/misc4dev/run_tests.pl
> --run-cypress-tests-only
>    - ERM/DataProviders_spec.ts and ERM/UsageReports_spec.ts failed, but I
> think they've been fixed now
>    - KohaTable/Holdings_spec.ts sometimes failed for me [2]
>    - KohaTable/KohaTable_spec.ts sometimes failed for me [3]

I think there are failing because you have not rebuilt the swapper spec (`yarn api:bundle` then restarted plack, or `reset_all`).
In theory they could still fail, but with way less reproducibility (without the  spec regenerated, branchcodes are generated using only 1 letter, and it clashes if it starts with the first letter of an existing library (eg. C and CPL in your case). With the new changes it will be 10 letters long and we won't have such clashes.

> 9. The tests related to bugs 40301 and 40320 failed for me.

I would suggest to have the first batch (up to bug 40181) pushed before we focus on the other ones :)
Comment 8 Jonathan Druart 2025-07-10 11:59:23 UTC
Created attachment 183929 [details] [review]
Bug 40174: Deal with enum

Data too long for column 'checkprevcheckout'
Comment 9 Jonathan Druart 2025-07-11 08:23:48 UTC
Created attachment 183978 [details] [review]
Bug 40174: Set minLength to 3

When we generate several objects we often get "Duplicate ID" from the
server. This mostly happens when the tests failed previously and the
objects have not been removed from the DB.
Comment 10 Jonathan Druart 2025-07-11 10:05:59 UTC
Created attachment 183980 [details] [review]
Bug 40174: Set minLength to 3

When we generate several objects we often get "Duplicate ID" from the
server. This mostly happens when the tests failed previously and the
objects have not been removed from the DB.
Comment 11 Jonathan Druart 2025-07-11 10:06:47 UTC
The second version of the last patch fixes mockData.ts
Comment 12 Jonathan Druart 2025-07-11 10:18:02 UTC
Created attachment 183981 [details] [review]
Bug 40174: Require RESTBasicAuth turned ON

From MM thread https://chat.koha-community.org/koha-community/pl/aptoodssm7gu8x491onfgxs1xr

Cypress tests need RESTBasicAuth - I’m stuck in a chicken-and-egg problem being eaten by its own tail...
I need advice. Some Cypress tests need RESTBasicAuth, but it's disabled by default.
My plan was to turn it on globally (ie. set before any tests, and reset to its original value after)
but we cannot right now:
Update in DB could work, but its cached
Call the svc script won't work, as we are not logged in yet (and this script require the session cookie)

so, I have a lot of solutions, but not sure which one to pick
* have a "flush cache" route (smells the rabbit hole)
* login to get the cookie, set the syspref via the svc script, logout (ugly, and will slower the cypress tests)
* require the pref on for cypress tests
* ?

I would opt now for the third option, turn it on in Jenkins when we run the cypress tests. And explode for devs when the pref is not set.
What do you think?

If we push this as it, Jenkins is going to fail.
We need to restructure how Jenkins is running the Cypress tests anyway:
we need a dedicated job to run them all. We will turn ON the syspref for
this job.
Comment 13 Matt Blenkinsop 2025-07-14 09:47:19 UTC
I'm getting a failure in KohaTable/KohaTable_spec.ts - looks like its to do with the patron category failing to render (line 647 patron-search.inc)
Comment 14 Jonathan Druart 2025-07-15 07:35:00 UTC
(In reply to Matt Blenkinsop from comment #13)
> I'm getting a failure in KohaTable/KohaTable_spec.ts - looks like its to do
> with the patron category failing to render (line 647 patron-search.inc)

Booh, I've the fix on the tree, but not attached here!
  Bug 40344: Fix KohaTable_spec.ts

Moving it here now.
Comment 15 Jonathan Druart 2025-07-15 07:35:58 UTC
(In reply to Jonathan Druart from comment #14)
> (In reply to Matt Blenkinsop from comment #13)
> > I'm getting a failure in KohaTable/KohaTable_spec.ts - looks like its to do
> > with the patron category failing to render (line 647 patron-search.inc)
> 
> Booh, I've the fix on the tree, but not attached here!
>   Bug 40344: Fix KohaTable_spec.ts
> 
> Moving it here now.

Hum it's on its own bug actually, adding the dependency instead then.
Comment 16 Matt Blenkinsop 2025-07-15 13:41:01 UTC
Created attachment 184103 [details] [review]
Bug 40174: Allow proper E2E testing with Cypress

This is the first step toward implementing true end-to-end testing with
Cypress.

Until now, we have been mocking responses using cy.intercept(), but this
approach can lead to confusion. In some cases, we have even had to mock
global JavaScript variables, which makes the code unnecessarily complex
and unconventional (e.g. win.categories_map).

All the bug reports listed under this tree have helped lay the
groundwork for this patch. With it, we are able to build mock objects
(via plugin mockData) and insert them directly into the database
(insertData). Once the tests are complete, we’ll restore the database to
its previous state by removing any data generated during the tests.

The tests in KohaTable/Holdings_spec.ts have been updated to leverage
this new setup.

Additionally, a caching mechanism has been added to prevent _id
attributes from being generated with the same values (a rare issue we
occasionally encountered).

While this is still not a perfect solution, it introduces a solid
foundation for future improvements.

Test plan:
All Cypress tests must pass

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 17 Matt Blenkinsop 2025-07-15 13:41:03 UTC
Created attachment 184104 [details] [review]
Bug 40174: Introduce Cypress auth plugin

So we can easily retrieve the value for the Authorization header.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 18 Matt Blenkinsop 2025-07-15 13:41:06 UTC
Created attachment 184105 [details] [review]
Bug 40174: Use api-client

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 19 Matt Blenkinsop 2025-07-15 13:41:08 UTC
Created attachment 184106 [details] [review]
Bug 40174: Deal with enum

Data too long for column 'checkprevcheckout'

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 20 Matt Blenkinsop 2025-07-15 13:41:11 UTC
Created attachment 184107 [details] [review]
Bug 40174: Set minLength to 3

When we generate several objects we often get "Duplicate ID" from the
server. This mostly happens when the tests failed previously and the
objects have not been removed from the DB.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 21 Matt Blenkinsop 2025-07-15 13:41:13 UTC
Created attachment 184108 [details] [review]
Bug 40174: Require RESTBasicAuth turned ON

From MM thread https://chat.koha-community.org/koha-community/pl/aptoodssm7gu8x491onfgxs1xr

Cypress tests need RESTBasicAuth - I’m stuck in a chicken-and-egg problem being eaten by its own tail...
I need advice. Some Cypress tests need RESTBasicAuth, but it's disabled by default.
My plan was to turn it on globally (ie. set before any tests, and reset to its original value after)
but we cannot right now:
Update in DB could work, but its cached
Call the svc script won't work, as we are not logged in yet (and this script require the session cookie)

so, I have a lot of solutions, but not sure which one to pick
* have a "flush cache" route (smells the rabbit hole)
* login to get the cookie, set the syspref via the svc script, logout (ugly, and will slower the cypress tests)
* require the pref on for cypress tests
* ?

I would opt now for the third option, turn it on in Jenkins when we run the cypress tests. And explode for devs when the pref is not set.
What do you think?

If we push this as it, Jenkins is going to fail.
We need to restructure how Jenkins is running the Cypress tests anyway:
we need a dedicated job to run them all. We will turn ON the syspref for
this job.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 22 Jonathan Druart 2025-07-17 12:53:13 UTC
Created attachment 184195 [details] [review]
Bug 40174: Do not generate bigint

Prevent "Out of range value for column 'deliverytime'" in a later
follow-up bug reports.
Comment 23 Martin Renvoize (ashimema) 2025-07-18 11:21:46 UTC
Created attachment 184327 [details] [review]
Bug 40174: Allow proper E2E testing with Cypress

This is the first step toward implementing true end-to-end testing with
Cypress.

Until now, we have been mocking responses using cy.intercept(), but this
approach can lead to confusion. In some cases, we have even had to mock
global JavaScript variables, which makes the code unnecessarily complex
and unconventional (e.g. win.categories_map).

All the bug reports listed under this tree have helped lay the
groundwork for this patch. With it, we are able to build mock objects
(via plugin mockData) and insert them directly into the database
(insertData). Once the tests are complete, we’ll restore the database to
its previous state by removing any data generated during the tests.

The tests in KohaTable/Holdings_spec.ts have been updated to leverage
this new setup.

Additionally, a caching mechanism has been added to prevent _id
attributes from being generated with the same values (a rare issue we
occasionally encountered).

While this is still not a perfect solution, it introduces a solid
foundation for future improvements.

Test plan:
All Cypress tests must pass

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 24 Martin Renvoize (ashimema) 2025-07-18 11:21:49 UTC
Created attachment 184328 [details] [review]
Bug 40174: Introduce Cypress auth plugin

So we can easily retrieve the value for the Authorization header.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 25 Martin Renvoize (ashimema) 2025-07-18 11:21:51 UTC
Created attachment 184329 [details] [review]
Bug 40174: Use api-client

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 26 Martin Renvoize (ashimema) 2025-07-18 11:21:53 UTC
Created attachment 184330 [details] [review]
Bug 40174: Deal with enum

Data too long for column 'checkprevcheckout'

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 27 Martin Renvoize (ashimema) 2025-07-18 11:21:56 UTC
Created attachment 184331 [details] [review]
Bug 40174: Set minLength to 3

When we generate several objects we often get "Duplicate ID" from the
server. This mostly happens when the tests failed previously and the
objects have not been removed from the DB.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 28 Martin Renvoize (ashimema) 2025-07-18 11:21:58 UTC
Created attachment 184332 [details] [review]
Bug 40174: Require RESTBasicAuth turned ON

From MM thread https://chat.koha-community.org/koha-community/pl/aptoodssm7gu8x491onfgxs1xr

Cypress tests need RESTBasicAuth - I’m stuck in a chicken-and-egg problem being eaten by its own tail...
I need advice. Some Cypress tests need RESTBasicAuth, but it's disabled by default.
My plan was to turn it on globally (ie. set before any tests, and reset to its original value after)
but we cannot right now:
Update in DB could work, but its cached
Call the svc script won't work, as we are not logged in yet (and this script require the session cookie)

so, I have a lot of solutions, but not sure which one to pick
* have a "flush cache" route (smells the rabbit hole)
* login to get the cookie, set the syspref via the svc script, logout (ugly, and will slower the cypress tests)
* require the pref on for cypress tests
* ?

I would opt now for the third option, turn it on in Jenkins when we run the cypress tests. And explode for devs when the pref is not set.
What do you think?

If we push this as it, Jenkins is going to fail.
We need to restructure how Jenkins is running the Cypress tests anyway:
we need a dedicated job to run them all. We will turn ON the syspref for
this job.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 29 Martin Renvoize (ashimema) 2025-07-18 11:22:01 UTC
Created attachment 184333 [details] [review]
Bug 40174: Do not generate bigint

Prevent "Out of range value for column 'deliverytime'" in a later
follow-up bug reports.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>