Bug 10270 - GetOrdersByBiblionumber needs better unit tests
Summary: GetOrdersByBiblionumber needs better unit tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.12
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on: 7593
Blocks: 10274 10575
  Show dependency treegraph
 
Reported: 2013-05-17 09:36 UTC by Jonathan Druart
Modified: 2014-12-07 20:03 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10270: GetOrdersByBiblionumber needs better unit tests (4.51 KB, patch)
2013-05-17 09:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10270: GetOrdersByBiblionumber needs better unit tests (4.60 KB, patch)
2013-05-17 09:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF][PATCH] Bug 10270: GetOrdersByBiblionumber needs better unit tests (4.81 KB, patch)
2013-05-17 10:13 UTC, Mathieu Saby
Details | Diff | Splinter Review
Bug 10270: GetOrdersByBiblionumber needs better unit tests (4.58 KB, patch)
2013-05-17 12:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF][PATCH] Bug 10270: GetOrdersByBiblionumber needs better unit tests (4.80 KB, patch)
2013-05-17 13:32 UTC, Mathieu Saby
Details | Diff | Splinter Review
Bug 10270: Fix shebang (739 bytes, patch)
2013-05-17 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 10270: GetOrdersByBiblionumber needs better unit tests (4.88 KB, patch)
2013-05-18 08:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10270: Fix shebang (805 bytes, patch)
2013-05-18 08:09 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 2013-05-17 09:36:22 UTC

    
Comment 1 Jonathan Druart 2013-05-17 09:40:41 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-05-17 09:48:08 UTC Comment hidden (obsolete)
Comment 3 Mathieu Saby 2013-05-17 09:51:42 UTC
Thank you Jonathan, that looks GREAT!
I am going to test it right now.

Mathieu
Comment 4 Mathieu Saby 2013-05-17 10:10:49 UTC
I made 2 tests :

$prove -v t/db_dependent/Acquisition/GetOrdersByBiblionumber.t
t/db_dependent/Acquisition/GetOrdersByBiblionumber.t .. 
ok 1 - GetOrdersByBiblionumber : no argument, return undef
ok 2 - 1 order on biblionumber 1
ok 3 - 2 orders on biblionumber 2
1..3
ok
All tests successful.
Files=1, Tests=3,  1 wallclock secs ( 0.02 usr  0.00 sys +  0.36 cusr  0.03 csys =  0.41 CPU)
Result: PASS

and

$prove -v t/db_dependent/Acquisition.t
t/db_dependent/Acquisition.t .. 
1..37
[...]
All tests successful.
Files=1, Tests=37,  1 wallclock secs ( 0.01 usr  0.01 sys +  0.36 cusr  0.04 csys =  0.42 CPU)
Result: PASS

I sign off.
Comment 5 Mathieu Saby 2013-05-17 10:13:41 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-05-17 12:56:39 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-05-17 12:57:13 UTC
typo in the last patch:

-use C4::Biblio qw( AddBiblio );
+use C4::Biblio;
[...]
-( undef, $ordernumber2 ) = C4::Acquisition::NewOrder(
+( undef, $ordernumber3 ) = C4::Acquisition::NewOrder(
Comment 8 Mathieu Saby 2013-05-17 12:58:40 UTC
However, it worked. Need to be tested again?

Mathieu
Comment 9 Jonathan Druart 2013-05-17 13:01:11 UTC
Needs SO
Comment 10 Mathieu Saby 2013-05-17 13:32:21 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2013-05-17 14:18:57 UTC
Jonathan, a question about:

+++ b/t/db_dependent/Acquisition/GetOrdersByBiblionumber.t
@@ -0,0 +1,84 @@
+#!/usr/bin/env perl

Why use env perl instead of perl? Note that we only use env perl in 6 Koha files (2 in the test directory t).
I understand that it is more portable. But then we should do it more often.. All our scripts contain the fixed perl path.
Just for consistency.
Comment 12 Jonathan Druart 2013-05-17 14:57:24 UTC
(In reply to comment #11)
> Jonathan, a question about:
> 
> +++ b/t/db_dependent/Acquisition/GetOrdersByBiblionumber.t
> @@ -0,0 +1,84 @@
> +#!/usr/bin/env perl
> 
> Why use env perl instead of perl? Note that we only use env perl in 6 Koha
> files (2 in the test directory t).
> I understand that it is more portable. But then we should do it more often..
> All our scripts contain the fixed perl path.
> Just for consistency.

There is no specific reason, I thought it is a good practice.
A patch coming...
Comment 13 Jonathan Druart 2013-05-17 14:58:09 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2013-05-18 08:08:53 UTC
Created attachment 18222 [details] [review]
[PASSED QA] Bug 10270: GetOrdersByBiblionumber needs better unit tests

prove t/db_dependent/Acquisition/GetOrdersByBiblionumber.t

Signed-off-by:Mathieu Saby <mathieu.saby@univ-rennes2.fr>
I made 2 tests :
$prove -v t/db_dependent/Acquisition/GetOrdersByBiblionumber.t
and
$prove -v t/db_dependent/Acquisition.t
The 2 tests are successful.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.
Comment 15 Katrin Fischer 2013-05-18 08:09:03 UTC
Created attachment 18223 [details] [review]
[PASSED QA] Bug 10270: Fix shebang

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 16 Jared Camins-Esakov 2013-05-18 10:14:20 UTC
This patch has been pushed to master and 3.12.x.