Bug 32304

Summary: Fix subtest search_limited and purge in BackgroundJobs.t
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Test SuiteAssignee: Marcel de Rooy <m.de.rooy>
Status: RESOLVED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, martin.renvoize, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00
Bug Depends on: 31969    
Bug Blocks:    
Attachments: Bug 32304: BackgroundJobs.t - Start purge test with clean sheet
Bug 32304: Fix subtest search_limited
Bug 32304: BackgroundJobs.t: Fix subtest purge
Bug 32304: Fix subtest search_limited
Bug 32304: BackgroundJobs.t: Fix subtest purge

Description Marcel de Rooy 2022-11-21 09:42:24 UTC
See comment18 of bug 32242
Comment 1 Marcel de Rooy 2022-11-21 09:55:41 UTC
Created attachment 144108 [details] [review]
Bug 32304: BackgroundJobs.t - Start purge test with clean sheet

Test plan:
Run t/db_dependent/Koha/BackgroundJobs.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2022-11-21 09:56:53 UTC
Trivial: self SO
Comment 3 Jonathan Druart 2022-11-21 10:23:53 UTC
It's usually a bad idea to remove data. You should adapt your code, like count before and after. We had problem recently (like delete from borrowers does not work if you have issues, etc.)
Comment 4 Marcel de Rooy 2022-11-21 10:31:31 UTC
(In reply to Jonathan Druart from comment #3)
> It's usually a bad idea to remove data. You should adapt your code, like
> count before and after. We had problem recently (like delete from borrowers
> does not work if you have issues, etc.)

Thanks.
I didnt write this test. So I leave that to the author. Just providing a quick fix for a 'silly' assumption.
Note that there are a lot of those deletes in test scripts though.
Comment 5 Jonathan Druart 2022-11-21 12:33:27 UTC
31969 is all wrong, see bug 31969 comment 18
Comment 6 Marcel de Rooy 2022-11-25 08:06:32 UTC
Tomás: What is your plan here?
Comment 7 Marcel de Rooy 2022-11-25 08:20:01 UTC
Created attachment 144227 [details] [review]
Bug 32304: Fix subtest search_limited

Jenkins reported:
    not ok 1 - No jobs found without userenv

We need to check if there are jobs without borrowernumber.

Test plan:
Run test again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2022-11-25 08:20:06 UTC
Created attachment 144228 [details] [review]
Bug 32304: BackgroundJobs.t: Fix subtest purge

Instead of deleting, I added a count :)
Note that one of the purges eventually deletes my
older jobs..

Test plan:
Run t/db_dependent/Koha/BackgroundJobs.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2022-11-25 08:20:35 UTC
Comment on attachment 144108 [details] [review]
Bug 32304: BackgroundJobs.t - Start purge test with clean sheet

To make Joubu happy ;)
Comment 10 Martin Renvoize 2022-11-25 08:32:21 UTC
Created attachment 144231 [details] [review]
Bug 32304: Fix subtest search_limited

Jenkins reported:
    not ok 1 - No jobs found without userenv

We need to check if there are jobs without borrowernumber.

Test plan:
Run test again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2022-11-25 08:32:25 UTC
Created attachment 144232 [details] [review]
Bug 32304: BackgroundJobs.t: Fix subtest purge

Instead of deleting, I added a count :)
Note that one of the purges eventually deletes my
older jobs..

Test plan:
Run t/db_dependent/Koha/BackgroundJobs.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2022-11-25 08:33:50 UTC
Confirmed fixes the failures, thanks Marcel.

Straight PQA from me.
Comment 13 Jonathan Druart 2022-11-25 08:43:51 UTC
Please see last patch on bug 31969.
Comment 14 Tomás Cohen Arazi 2022-11-25 12:46:50 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 15 Jonathan Druart 2022-11-25 12:57:24 UTC
No idea why this has been pushed right before the follow-up on bug 31969 (Use filter_by_last_update) that actually... removes this code.