Bug 36924 - t/db_dependent/Search.t generates warnings
Summary: t/db_dependent/Search.t generates warnings
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25515
  Show dependency treegraph
 
Reported: 2024-05-22 12:42 UTC by Jonathan Druart
Modified: 2024-05-31 15:16 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12
Circulation function:


Attachments
Bug 36924: Remove "I don't know what to say" warnings (1.06 KB, patch)
2024-05-22 12:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36924: Remove "I don't know what to say" warnings (1.15 KB, patch)
2024-05-22 13:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 36924: (follow-up) Remove unused $transfer (1.39 KB, patch)
2024-05-22 13:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item" (2.33 KB, patch)
2024-05-23 06:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36924: Remove "I don't know what to say" warnings (1.16 KB, patch)
2024-05-23 07:04 UTC, David Nind
Details | Diff | Splinter Review
Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item" (2.37 KB, patch)
2024-05-23 07:04 UTC, David Nind
Details | Diff | Splinter Review
Bug 36924: Remove "I don't know what to say" warnings (1.25 KB, patch)
2024-05-23 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item" (2.58 KB, patch)
2024-05-23 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-05-22 12:42:22 UTC
Value not allowed for auto_incr itemnumber in Item at /kohadevbox/koha/t/lib/TestBuilder.pm line 402.
The syspref AuthorLinkSortBy was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 163.
The syspref AuthorLinkSortOrder was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 163.
The syspref COinSinOPACResults was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 163.
The syspref AuthorLinkSortBy was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 163.
The syspref AuthorLinkSortOrder was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 163.
The syspref COinSinOPACResults was requested but I don't know what to say; this indicates that the test requires updating at t/db_dependent/Search.t line 163.
Comment 1 Jonathan Druart 2024-05-22 12:52:44 UTC
Created attachment 167028 [details] [review]
Bug 36924: Remove "I don't know what to say" warnings

Those are a pain really
Comment 2 Jonathan Druart 2024-05-22 12:53:43 UTC
Marcel, any ideas about how to remove the remaining warning?

The item with itemnumber=11 does not even exist in the DB...
Comment 3 Marcel de Rooy 2024-05-22 13:39:11 UTC
(In reply to Jonathan Druart from comment #2)
> Marcel, any ideas about how to remove the remaining warning?
> 
> The item with itemnumber=11 does not even exist in the DB...

Shouldnt we just add a sample item here?
Comment 4 Marcel de Rooy 2024-05-22 13:40:13 UTC
This $transfer is unused ??
Comment 5 Marcel de Rooy 2024-05-22 13:43:08 UTC
Created attachment 167040 [details] [review]
Bug 36924: Remove "I don't know what to say" warnings

Those are a pain really

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2024-05-22 13:43:10 UTC
Created attachment 167041 [details] [review]
Bug 36924: (follow-up) Remove unused $transfer

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2024-05-22 13:58:24 UTC
(In reply to Marcel de Rooy from comment #6)
> Created attachment 167041 [details] [review] [review]
> Bug 36924: (follow-up) Remove unused $transfer
> 
> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

This is what I've tried first but actually I'm my understanding we are removing the test. We want to make sure it is still working if a transfer exists.
Comment 8 Marcel de Rooy 2024-05-22 14:14:51 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to Marcel de Rooy from comment #6)
> > Created attachment 167041 [details] [review] [review] [review]
> > Bug 36924: (follow-up) Remove unused $transfer
> > 
> > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> 
> This is what I've tried first but actually I'm my understanding we are
> removing the test. We want to make sure it is still working if a transfer
> exists.

Having another look. But looked like it didnt make much sense..
Comment 9 Marcel de Rooy 2024-05-22 14:38:17 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to Marcel de Rooy from comment #6)
> > Created attachment 167041 [details] [review] [review] [review]
> > Bug 36924: (follow-up) Remove unused $transfer
> > 
> > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> 
> This is what I've tried first but actually I'm my understanding we are
> removing the test. We want to make sure it is still working if a transfer
> exists.

We load the records in a mocked Zebra, but they do not exist in db. Now we are adding a transfer to an item that is not there. Scrolling thru C4/Search, I am wondering if we do not have other calls that miss a referenced record too (biblio or item etc).

So if you want to really fix this, you will need to add some items and manipulate the marcresults that you pass to searchResults and update those itemnumbers.

I think that the test in its current form has very limited value (in view of the mocks and bad data). Fixing that is kind of advanced hacking. I opt for removing it. Will remove my follow-up for now.

We should certainly not update TestBuilder for this! We could hide the warning but it is useless imo.
Comment 10 Jonathan Druart 2024-05-23 06:17:37 UTC
I agree that it's too time consuming to adjust this fragile test and that we can remove it. However we should adjust the code around. Removing the test entirely and the comment?
Comment 11 Marcel de Rooy 2024-05-23 06:18:44 UTC
(In reply to Jonathan Druart from comment #10)
> I agree that it's too time consuming to adjust this fragile test and that we
> can remove it. However we should adjust the code around. Removing the test
> entirely and the comment?

Yes, the immediate context too.
Comment 12 Jonathan Druart 2024-05-23 06:50:40 UTC
Created attachment 167064 [details] [review]
Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item"

We are removing entirely the badly written test.
Comment 13 David Nind 2024-05-23 07:04:28 UTC
Created attachment 167067 [details] [review]
Bug 36924: Remove "I don't know what to say" warnings

Those are a pain really

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Comment 14 David Nind 2024-05-23 07:04:31 UTC
Created attachment 167068 [details] [review]
Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item"

We are removing entirely the badly written test.

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 Marcel de Rooy 2024-05-23 07:16:18 UTC
Created attachment 167070 [details] [review]
Bug 36924: Remove "I don't know what to say" warnings

Those are a pain really

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2024-05-23 07:16:21 UTC
Created attachment 167071 [details] [review]
Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item"

We are removing entirely the badly written test.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Note: We cannot test this properly when the search index refers to biblios
and items that do not exist in the database.
Comment 17 Katrin Fischer 2024-05-23 15:57:04 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 18 Fridolin Somers 2024-05-29 14:42:41 UTC
Pushed to 23.11.x for 23.11.06
Comment 19 Lucas Gass (lukeg) 2024-05-31 15:16:37 UTC
Backported to 23.05.x for upcoming 23.05.12