[Bug 26364] New: XISBN.t makes a bad assumption about return values
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Bug ID: 26364 Summary: XISBN.t makes a bad assumption about return values Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org 75 eval { $results_thingisbn = C4::XISBN::get_xisbns($isbn1,$biblionumber3); }; 76 SKIP: { 77 skip "Problem retrieving ThingISBN", 1 78 unless $@ eq ''; 79 is( $results_thingisbn->[0]->{biblionumber}, 80 undef, 81 "Doesn't get biblionumber if the biblionumber matches the one passed to the sub." ); 82 } Here we are testing that we don't get the same book that we passed into the sub in the list that is returned, however, we shouldn't assume that we get nothing, we should just check that it is not the biblionumber we passed in -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109599 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109599&action=edit Bug 26364: Check that return value isn't the value we don't want. Don't check for undef It seems sometimes we get an ISBN returned that matches a book in the sample data. One of the Harry Potter books was matching the ISBN. Our real concern is not that we get nothing, but that we don't get the same value we pass in. We should test for that To test: 1 - prove -v t/db_dependent/XISBN.t 2 - The failures are not consistent, so read the code and ensure we are making sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |nick@bywatersolutions.com --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Can you explain? Is it an alternative patch for bug 26270? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Where did you see it fail? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #3)
Where did you see it fail?
It was failing on our package builds, but not locally It seemed that the ISBN previously returned nothing, but now had an ISBN that matched one in our system The change makes sense though, we just need to confirm we don't get the biblionumber, not that we get nothing ThingISBN can change and makes no guarantees, so our tests must be explicit -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Nick Clemens from comment #4)
(In reply to Jonathan Druart from comment #3)
Where did you see it fail?
It was failing on our package builds, but not locally
It seemed that the ISBN previously returned nothing, but now had an ISBN that matched one in our system
The change makes sense though, we just need to confirm we don't get the biblionumber, not that we get nothing
ThingISBN can change and makes no guarantees, so our tests must be explicit
bug 26270 skips if the normalized ISBNs are equal to the xisbns it should only do so if the biblionumber is the same - isbn_bib1 = 0590353403 isbn_bib2 = 0684843897 isbn_bib3 = 043936213X i.e. we send 043936213X, thing ISBN returns 0590353403 with 26270 we skip the results because normalized 0590353403 = 0590353403 but that is ok the test before failed because it said 043936213X returned nothing ThingISBN now returns 0590353403 for 043936213X so we get bib1 - as long as we don't get bib3 then things are working -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109599|0 |1 is obsolete| | --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109631 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109631&action=edit Bug 26364: Check that return value isn't the value we don't want. Don't check for undef It seems sometimes we get an ISBN returned that matches a book in the sample data. One of the Harry Potter books was matching the ISBN. Our real concern is not that we get nothing, but that we don't get the same value we pass in. We should test for that To test: 1 - prove -v t/db_dependent/XISBN.t 2 - The failures are not consistent, so read the code and ensure we are making sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |26270 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26270 [Bug 26270] XISBN.t is failing since today -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> ---
2 - The failures are not consistent, so read the code and ensure we are making sense
No, the failure is consistent if you don't have bug 26270. Nick, did you understand why this test start failing? I guess the service changes their return value. Do you know what has changed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109631|0 |1 is obsolete| | --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109821 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109821&action=edit Bug 26364: Don't skip records that match the isbn we passed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109822 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109822&action=edit Bug 26364: Remove false matches on isbn The test here was returning 0590353403 when searched for 9780590353403 because of the regex matching Koha doesn't work this way unless SearchWithISBNVariations is set to 'search' and you use a dropdown to select ISBN for searching To test: 1 - Search catalog for nb=9780590353403 2 - Confirm you don't have results, or delete the records with results 3 - Add isbn 0590353403 to a record 4 - Repeat search, fails 5 - Enable SearchWithISBNVariations 6 - Repeat search, fails 7 - Go to advanced search, select ISBN, search for 9780590353403 8 - Get results -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109823&action=edit Bug 26364: Remove useless condition $response_data is a hash with one key, content Why are we checkiing it against a string? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109824&action=edit Bug 26364: Don't discard the passed isbn I don't know, if we get rid fo this check we can now check the db for the same isbn we passed This may be useful as we may want to see the biblio with the same isbn as long as number is different? This will make tests fail -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109825&action=edit Bug 26364: Adjust unit tests We shouldn't rely on knowing exactly which ISBNs we get, we should specifically check for what we do or don't expect. I believe we should return biblios that have the same isbn we passed, as it signlas we have another biblio in the catalog that matches the one we are on. To test: prove -v t/db_dependent/XISBN.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- To test: 1 - Apply patch 1 2 - prove -v t/db_dependent/XISBN.t 3 - test fails 4 - Apply patch 2 5 - tests pass 6 - Apply patch 3 - no change 7 - Apply patch 4 - tests fail 8 - Apply patch 5 - tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109821|0 |1 is obsolete| | --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 113054 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113054&action=edit Bug 26364: Don't skip records that match the isbn we passed Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109822|0 |1 is obsolete| | --- Comment #15 from David Nind <david@davidnind.com> --- Created attachment 113055 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113055&action=edit Bug 26364: Remove false matches on isbn The test here was returning 0590353403 when searched for 9780590353403 because of the regex matching Koha doesn't work this way unless SearchWithISBNVariations is set to 'search' and you use a dropdown to select ISBN for searching To test: 1 - Search catalog for nb=9780590353403 2 - Confirm you don't have results, or delete the records with results 3 - Add isbn 0590353403 to a record 4 - Repeat search, fails 5 - Enable SearchWithISBNVariations 6 - Repeat search, fails 7 - Go to advanced search, select ISBN, search for 9780590353403 8 - Get results Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109823|0 |1 is obsolete| | --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 113056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113056&action=edit Bug 26364: Remove useless condition $response_data is a hash with one key, content Why are we checkiing it against a string? Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109824|0 |1 is obsolete| | --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 113057 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113057&action=edit Bug 26364: Don't discard the passed isbn I don't know, if we get rid fo this check we can now check the db for the same isbn we passed This may be useful as we may want to see the biblio with the same isbn as long as number is different? This will make tests fail Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109825|0 |1 is obsolete| | --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 113058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113058&action=edit Bug 26364: Adjust unit tests We shouldn't rely on knowing exactly which ISBNs we get, we should specifically check for what we do or don't expect. I believe we should return biblios that have the same isbn we passed, as it signlas we have another biblio in the catalog that matches the one we are on. To test: prove -v t/db_dependent/XISBN.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #19 from David Nind <david@davidnind.com> --- To test, I followed the test plans in comment #13, #15 and #18. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113054|0 |1 is obsolete| | --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 114818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114818&action=edit Bug 26364: Don't skip records that match the isbn we passed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113055|0 |1 is obsolete| | --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 114819 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114819&action=edit Bug 26364: Remove false matches on isbn The test here was returning 0590353403 when searched for 9780590353403 because of the regex matching Koha doesn't work this way unless SearchWithISBNVariations is set to 'search' and you use a dropdown to select ISBN for searching To test: 1 - Search catalog for nb=9780590353403 2 - Confirm you don't have results, or delete the records with results 3 - Add isbn 0590353403 to a record 4 - Repeat search, fails 5 - Enable SearchWithISBNVariations 6 - Repeat search, fails 7 - Go to advanced search, select ISBN, search for 9780590353403 8 - Get results Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113056|0 |1 is obsolete| | --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 114820 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114820&action=edit Bug 26364: Remove useless condition $response_data is a hash with one key, content Why are we checkiing it against a string? Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113057|0 |1 is obsolete| | --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 114821 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114821&action=edit Bug 26364: Don't discard the passed isbn I don't know, if we get rid fo this check we can now check the db for the same isbn we passed This may be useful as we may want to see the biblio with the same isbn as long as number is different? This will make tests fail Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113058|0 |1 is obsolete| | --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 114822 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114822&action=edit Bug 26364: Adjust unit tests We shouldn't rely on knowing exactly which ISBNs we get, we should specifically check for what we do or don't expect. I believe we should return biblios that have the same isbn we passed, as it signlas we have another biblio in the catalog that matches the one we are on. To test: prove -v t/db_dependent/XISBN.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #7)
2 - The failures are not consistent, so read the code and ensure we are making sense
No, the failure is consistent if you don't have bug 26270.
Nick, did you understand why this test start failing? I guess the service changes their return value. Do you know what has changed?
I am not sure if this question was answered on another channel. Nick, can you please comment? Patches apply and QA tests pass. Unit tests as well. -- You are receiving this mail because: You are watching all bug changes.
(In reply to Jonathan Druart from comment #7)
Nick, did you understand why this test start failing? I guess the service changes their return value. Do you know what has changed?
I am not sure if this question was answered on another channel. Nick, can you please comment? comment #5: ThingISBN now returns 0590353403 for 043936213X so we get a match we didn't get
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #26 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #25) previously -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|21.05.00 |21.05.00,20.11.02 released in| | Status|Pushed to master |Pushed to stable --- Comment #28 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable Version(s)|21.05.00,20.11.02 |21.05.00,20.11.02,20.05.08 released in| | --- Comment #29 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |victor@tuxayo.net Status|Pushed to oldstable |RESOLVED --- Comment #30 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26364 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28288 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28288 [Bug 28288] XISBN.t is failing is 500 is returned by the webservice -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org