[Bug 16582] New: t/Price.t test should pass if Test::DBIx::Class is not available
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Bug ID: 16582 Summary: t/Price.t test should pass if Test::DBIx::Class is not available 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: mtompset@hotmail.com QA Contact: gmcharlt@gmail.com I just spent hours doing git clone to attempt the install pain under Ubuntu 16.04 LTS to see what the problem is, and the tests were still bugging me. I'm going to attach 2 commits. The first is just to get the load test to keep going, and should not be used. The second will fill the t/Price.t in a manner similar to t/Members_AttributeTypes.t in bug 15150 was fixed. TEST PLAN --------- 1) try prove t -- load will bomb 2) apply first patch -- prices will fail 3) apply second patch -- prices will "succeed" 4) run koha qa test tools -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=15150 Assignee|chris@bigballofwax.co.nz |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 51758 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51758&action=edit Bug 16582 t/Price.t test should pass if Test::DBIx::Class is not available This patch makes it so 00-load.t doesn't cause 'prove t' to prematurely die before reaching other tests. TEST PLAN --------- 1) prove t -- dies on 00-load.t 2) apply patch 3) prove t -- now t/Prices.t should be the failure. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 51759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51759&action=edit Bug 16582 t/Price.t test should pass if Test::DBIx::Class is not available TEST PLAN --------- 1) prove t/Prices.t -- failure 2) apply patch 3) prove t/Prices.t -- nicely skipped when Test::DBIx::Class is not available. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.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=16582 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=16582 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51758|0 |1 is obsolete| | Attachment #51759|0 |1 is obsolete| | --- Comment #3 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 51909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51909&action=edit [SIGNED-OFF] Bug 16582 t/Price.t test should pass if Test::DBIx::Class is not available This patch makes it so 00-load.t doesn't cause 'prove t' to prematurely die before reaching other tests. TEST PLAN --------- 1) prove t -- dies on 00-load.t 2) apply patch 3) prove t -- now t/Prices.t should be the failure. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Srdjan Jankovic <srdjan@catalyst.net.nz> 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=16582 --- Comment #4 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 51910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51910&action=edit [SIGNED-OFF] Bug 16582 t/Price.t test should pass if Test::DBIx::Class is not available TEST PLAN --------- 1) prove t/Prices.t -- failure 2) apply patch 3) prove t/Prices.t -- nicely skipped when Test::DBIx::Class is not available. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #5 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Just a couple of generic questions/musings not entirely to do with this patch: * Wouldn't it be better to rename tests that we don't want to .t.NOT_READY_YET or similar instead of doing those regexes in load.t? * I don't understand why use_ok are in the BEGIN block? * maybe (if not too much of a hassle) we could leave use_ok for Test::DBIx::Class dependant tests, and skip the rest? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Srdjan Jankovic from comment #5)
Just a couple of generic questions/musings not entirely to do with this patch:
* Wouldn't it be better to rename tests that we don't want to .t.NOT_READY_YET or similar instead of doing those regexes in load.t?
We should fix the tests or put some conditional skip in those tests. I don't favor the regexs in load.t. PS A new name for the first patch would be helpful. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #6)
We should fix the tests or put some conditional skip in those tests. I don't favor the regexs in load.t. Well probably not possible,,
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51909|0 |1 is obsolete| | --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 51913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51913&action=edit Bug 16582 [DO NOT PUSH] 'prove t' failure on 00-load.t This patch makes it so 00-load.t doesn't cause 'prove t' to prematurely die before reaching other tests. TEST PLAN --------- 1) prove t -- dies on 00-load.t 2) apply patch 3) prove t -- now t/Prices.t should be the failure. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51910|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 51914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51914&action=edit Bug 16582 t/Price.t test should pass if Test::DBIx::Class is not available TEST PLAN --------- 1) prove t/Prices.t -- failure 2) apply patch 3) prove t/Prices.t -- nicely skipped when Test::DBIx::Class is not available. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- As per comment #6's PS, first one is renamed. As per comment #5: "I don't understand why use_ok are in the BEGIN block?" -- I wondered the same thing. "A BEGIN code block is executed as soon as possible, that is, the moment it is completely defined, even before the rest of the containing file (or string) is parsed. You may have multiple BEGIN blocks within a file (or eval'ed string); they will execute in order of definition. Because a BEGIN code block executes immediately, it can pull in definitions of subroutines and such from other files in time to be visible to the rest of the compile and run time. Once a BEGIN has run, it is immediately undefined and any code it used is returned to Perl's memory pool." So, perhaps a memory optimization? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16618 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51913|0 |1 is obsolete| | --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 51913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51913 Bug 16582 [DO NOT PUSH] 'prove t' failure on 00-load.t While similar to bug 9054, regular expressions like this don't actually show the real problem of missing dependencies. And if those dependencies are completely optional, we shouldn't even test. This is handled better on bug 16618 now. No need to keep this first patch around. The second can be tested directly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Why not move this test to dependent btw? Seeing several warns on Context and one on Templates. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Marcel de Rooy from comment #12)
Why not move this test to dependent btw? Seeing several warns on Context and one on Templates.
What warns? I see a bunch of mock modules and functions. If I knew what you were refering to, then perhaps I would understand why you said this. Otherwise, with the mocks in place, it should be where it is. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to M. Tompsett from comment #13)
(In reply to Marcel de Rooy from comment #12)
Why not move this test to dependent btw? Seeing several warns on Context and one on Templates.
What warns? I see a bunch of mock modules and functions. If I knew what you were refering to, then perhaps I would understand why you said this. Otherwise, with the mocks in place, it should be where it is.
Run the test. I see these warnings from Context.pm (and Templates.pm) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #15 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Marcel de Rooy from comment #14)
Run the test. I see these warnings from Context.pm (and Templates.pm)
What warnings? Perhaps I am lacking data to trigger them? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to M. Tompsett from comment #15)
(In reply to Marcel de Rooy from comment #14)
Run the test. I see these warnings from Context.pm (and Templates.pm)
What warnings? Perhaps I am lacking data to trigger them?
Remove your KOHA_CONF since you are supposed to have no database? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #16)
(In reply to M. Tompsett from comment #15)
(In reply to Marcel de Rooy from comment #14)
Run the test. I see these warnings from Context.pm (and Templates.pm)
What warnings? Perhaps I am lacking data to trigger them?
Remove your KOHA_CONF since you are supposed to have no database?
from the env :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 --- Comment #18 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Marcel de Rooy from comment #16)
(In reply to M. Tompsett from comment #15)
(In reply to Marcel de Rooy from comment #14)
Run the test. I see these warnings from Context.pm (and Templates.pm)
What warnings? Perhaps I am lacking data to trigger them?
Remove your KOHA_CONF since you are supposed to have no database?
It seems that 'prove t' demonstrates this is a larger problem than just Price.t I would recommend moving that issue to another bug report, and moving forward with this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16622 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51914|0 |1 is obsolete| | --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51941&action=edit Bug 16582 t/Price.t test should pass if Test::DBIx::Class is not available TEST PLAN --------- 1) prove t/Prices.t -- failure 2) apply patch 3) prove t/Prices.t -- nicely skipped when Test::DBIx::Class is not available. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |brendan@bywatersolutions.co | |m Status|Passed QA |Pushed to Master --- Comment #20 from Brendan Gallagher <brendan@bywatersolutions.com> --- Pushed to Master - Should be in the November 2016 release. Thanks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #21 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.01. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16582 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |julian.maurice@biblibre.com --- Comment #22 from Julian Maurice <julian.maurice@biblibre.com> --- Patch pushed to 3.22.x, will be in 3.22.8 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org