Created attachment 51437 [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
Created attachment 51770 [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>
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.
(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.
Created attachment 51851 [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 quickly fixes it. Report 16607 will propose a general solution. Test plan: Do not yet apply the patch. Add MPL branch. Run the test. Notice the warning about Branch. Apply the patch. Run the test. Warning gone? Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 52229 [details] [review] [COUNTER-PATCH] Bug 16500: Do not try to insert MPL if already exists
(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?
(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..
Created attachment 52300 [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>
(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.
Created attachment 52335 [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>
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>
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>
Pushed to master for Koha 16.11, thanks Marcel!
Pushed in 16.05. Will be in 16.05.01.