Bug 26270 - XISBN.t is failing since today
Summary: XISBN.t is failing since today
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 26364
  Show dependency treegraph
 
Reported: 2020-08-21 12:00 UTC by Jonathan Druart
Modified: 2021-06-14 21:31 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.04, 19.11.10, 19.05.15


Attachments
Bug 26270: Fix ThingISBN after a change in behaviour (1.05 KB, patch)
2020-08-21 12:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26270: Fix ThingISBN after a change in behaviour (1.12 KB, patch)
2020-08-21 14:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26270: Fix ThingISBN after a change in behaviour (1.17 KB, patch)
2020-08-21 22:04 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-08-21 12:00:43 UTC
#   Failed test 'Doesn't get biblionumber if the biblionumber matches the one passed to the sub.'
#   at t/db_dependent/XISBN.t line 81.
#          got: '767'
#     expected: undef
# Looks like you failed 1 test of 5.
t/db_dependent/XISBN.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/5 subtests
Comment 1 Jonathan Druart 2020-08-21 12:02:08 UTC
Created attachment 108811 [details] [review]
Bug 26270: Fix ThingISBN after a change in behaviour

This is just a guess! One of our tests is failing since today and we
don't modify anything in this area.
My guess is that the service is now returning the normalized version of
the ISBN
Comment 2 Jonathan Druart 2020-08-21 12:40:58 UTC
Hypothetical test plan:
Create a bib record A with isbn
Create a bib record B with the normalized version of the isbn

Without this patch, on A you should see B
With this patch applied you should not see B

(I did not test)
Comment 3 Martin Renvoize 2020-08-21 14:24:50 UTC
Created attachment 108828 [details] [review]
Bug 26270: Fix ThingISBN after a change in behaviour

This is just a guess! One of our tests is failing since today and we
don't modify anything in this area.
My guess is that the service is now returning the normalized version of
the ISBN

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2020-08-21 14:25:21 UTC
I concur with your guess and I can't see that this patch would cause any regressions.

Signing off.
Comment 5 Katrin Fischer 2020-08-21 22:03:11 UTC
prove t/db_dependent/XISBN.t passes with the patch, not sure what else to test?
Comment 6 Katrin Fischer 2020-08-21 22:04:36 UTC
Created attachment 108874 [details] [review]
Bug 26270: Fix ThingISBN after a change in behaviour

This is just a guess! One of our tests is failing since today and we
don't modify anything in this area.
My guess is that the service is now returning the normalized version of
the ISBN

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Marcel de Rooy 2020-08-24 08:44:32 UTC
Use of uninitialized value $biblionumber in string ne at /usr/share/koha/C4/XISBN.pm line 111.
Comment 8 Jonathan Druart 2020-08-24 10:34:08 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Jonathan Druart 2020-08-24 10:35:55 UTC
(In reply to Marcel de Rooy from comment #7)
> Use of uninitialized value $biblionumber in string ne at
> /usr/share/koha/C4/XISBN.pm line 111.

It appeared before this patch.
Comment 10 Marcel de Rooy 2020-08-24 11:37:08 UTC
(In reply to Jonathan Druart from comment #8)
> Pushed to master for 20.11, thanks to everybody involved!

Ok, no need to look further here ;)
Comment 11 Jonathan Druart 2020-08-24 11:50:24 UTC
(In reply to Marcel de Rooy from comment #10)
> (In reply to Jonathan Druart from comment #8)
> > Pushed to master for 20.11, thanks to everybody involved!
> 
> Ok, no need to look further here ;)

I wanted to fix the failing test, but I think it would be good to confirm the hypothesis. I did not manage to validate it.
Comment 12 Aleisha Amohia 2020-09-01 22:33:19 UTC
backported to 19.11.x for 19.11.10
Comment 13 Lucas Gass 2020-09-02 14:51:16 UTC
backported to 20.05.x for 20.05.04
Comment 14 Victor Grousset/tuxayo 2020-09-07 17:29:40 UTC
Backported to 19.05.x branch for 19.05.15