Bug 16500 - Catch two warns in TestBuilder.t with warning_like
Summary: Catch two warns in TestBuilder.t with warning_like
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marcel de Rooy
QA Contact: Galen Charlton
URL:
Keywords:
Depends on: 16155
Blocks:
  Show dependency treegraph
 
Reported: 2016-05-11 14:10 UTC by Marcel de Rooy
Modified: 2017-12-07 22:17 UTC (History)
5 users (show)

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


Attachments
Bug 16500: Catch two warns in TestBuilder.t with warning_like (1.69 KB, patch)
2016-05-11 14:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16500: Catch two warns in TestBuilder.t with warning_like (1.70 KB, patch)
2016-05-25 12:33 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 16500: [QA Follow-up] Catch warning if branch MPL already exists (1.43 KB, patch)
2016-05-26 20:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[COUNTER-PATCH] Bug 16500: Do not try to insert MPL if already exists (1.16 KB, patch)
2016-06-10 14:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16500: [QA Follow-up] Catch warning if branch MPL already exists (1.51 KB, patch)
2016-06-13 07:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16500: Catch two warns in TestBuilder.t with warning_like (1.77 KB, patch)
2016-06-13 16:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16500: Catch two warns in TestBuilder.t with warning_like (1.77 KB, patch)
2016-06-13 16:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16500: [QA Follow-up] Catch warning if branch MPL already exists (1.54 KB, patch)
2016-06-13 16:50 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-05-11 14:10:15 UTC

    
Comment 1 Marcel de Rooy 2016-05-11 14:16:03 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2016-05-25 12:33:21 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-05-25 16:57:51 UTC
I get 3 without the patch and 1 remains with the patch applied, is it expected?

t/db_dependent/TestBuilder.t .. 1/9 Violation of unique constraint in Branch at t/lib/TestBuilder.pm line 201.
Comment 4 Marcel de Rooy 2016-05-26 19:08:46 UTC
(In reply to Jonathan Druart from comment #3)
> I get 3 without the patch and 1 remains with the patch applied, is it
> expected?
> 
> t/db_dependent/TestBuilder.t .. 1/9 Violation of unique constraint in Branch
> at t/lib/TestBuilder.pm line 201.

Thanks.

$ prove t/db_dependent/TestBuilder.t
t/db_dependent/TestBuilder.t .. ok
All tests successful.
Files=1, Tests=9,  7 wallclock secs ( 0.06 usr  0.00 sys +  6.48 cusr  0.13 csys =  6.67 CPU)
Result: PASS

I had no warnings, because I had no MPL branch. And the patch only catches the second build on line 57. If you do have one, the warning comes from line 56.
Will send a small follow-up.
Comment 5 Marcel de Rooy 2016-05-26 20:07:51 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-06-10 14:13:02 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-06-10 14:14:20 UTC
(In reply to Jonathan Druart from comment #6)
> Created attachment 52229 [details] [review] [review]
> [COUNTER-PATCH] Bug 16500: Do not try to insert MPL if already exists

Marcel, what about this patch to replace your second patch?
Comment 8 Marcel de Rooy 2016-06-10 18:50:38 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to Jonathan Druart from comment #6)
> > Created attachment 52229 [details] [review] [review] [review]
> > [COUNTER-PATCH] Bug 16500: Do not try to insert MPL if already exists
> 
> Marcel, what about this patch to replace your second patch?

That would obviously work fine too, but I'd rather replace it by one line in the dependent patch (16607). So I prefer to keep the original one..
Comment 9 Marcel de Rooy 2016-06-13 07:11:35 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2016-06-13 07:12:36 UTC
(In reply to Marcel de Rooy from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > (In reply to Jonathan Druart from comment #6)
> > > Created attachment 52229 [details] [review] [review] [review] [review]
> > > [COUNTER-PATCH] Bug 16500: Do not try to insert MPL if already exists
> > 
> > Marcel, what about this patch to replace your second patch?
> 
> That would obviously work fine too, but I'd rather replace it by one line in
> the dependent patch (16607). So I prefer to keep the original one..

I finally decided to go for the third patch :) and remove MPL completely.
Comment 11 Jonathan Druart 2016-06-13 16:49:04 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-06-13 16:49:42 UTC
Created attachment 52336 [details] [review]
Bug 16500: Catch two warns in TestBuilder.t with warning_like

When testing a wrong case in TestBuilder.t, two warns show up.
We can catch them with warning_like.

Test plan:
Run t/db_dependent/TestBuilder.t

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2016-06-13 16:50:37 UTC
Created attachment 52337 [details] [review]
Bug 16500: [QA Follow-up] Catch warning if branch MPL already exists

The current test had no warnings, when you have no MPL branch, and only
catches the warn from the second build on line 57. If you do have MPL,
a warning comes from line 56.

This patch fixes it by removing MPL from the test. A new branch is created
first and then we try to create it again.

Test plan:
Run the test.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Kyle M Hall 2016-06-17 14:42:50 UTC
Pushed to master for Koha 16.11, thanks Marcel!
Comment 15 Frédéric Demians 2016-06-24 05:45:45 UTC
Pushed in 16.05. Will be in 16.05.01.