Bug 18851 - Use Test::DBIx::Class in tests breaks packaging
Summary: Use Test::DBIx::Class in tests breaks packaging
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 18226
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-23 23:26 UTC by Mirko Tietgen
Modified: 2019-10-14 19:57 UTC (History)
7 users (show)

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


Attachments
Bug 18851: Database handler must not be created at runtime (2.13 KB, patch)
2017-07-05 20:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18851: Test::DBIx::Class is not needed in 00-load.t (724 bytes, patch)
2017-07-05 20:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed (1.16 KB, patch)
2017-07-05 20:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
build log: tests (15.40 KB, text/plain)
2017-07-07 13:45 UTC, Mirko Tietgen
Details
Bug 18851: Fix 3 other tests (2.23 KB, patch)
2017-07-10 19:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18851: Database handler must not be created at runtime (2.18 KB, patch)
2017-07-11 10:58 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 18851: Test::DBIx::Class is not needed in 00-load.t (773 bytes, patch)
2017-07-11 10:58 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed (1.21 KB, patch)
2017-07-11 10:58 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 18851: Fix 3 other tests (2.27 KB, patch)
2017-07-11 10:58 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 18851: Make OverDrive.T pass if dbms is not running (1.25 KB, patch)
2017-07-24 15:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18851: Fix few other tests (2.85 KB, patch)
2017-07-31 16:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18851: Database handler must not be created at runtime (2.24 KB, patch)
2017-08-07 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18851: Test::DBIx::Class is not needed in 00-load.t (828 bytes, patch)
2017-08-07 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed (1.26 KB, patch)
2017-08-07 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18851: Make OverDrive.T pass if dbms is not running (1.30 KB, patch)
2017-08-07 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18851: Fix few other tests (2.90 KB, patch)
2017-08-07 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18851: (QA followup) Test doesn't need sessions on DB (1023 bytes, patch)
2017-08-07 15:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Mirko Tietgen 2017-06-23 23:30:04 UTC
Bug 18226 introduces a regression for a problem fixed in Bug 15150.

Test::DBIx::Class is not mandatory because it is not available in Debian, packaging of Koha is broken because of the use in t/00-load.t.
Comment 2 Jonathan Druart 2017-07-05 20:10:02 UTC
My bad, I completely forgot that this module was not packaged.
So we have a problem here, the way we mocked our DB was not reliable and we fixed it using Test::DBIx::Class.

Olli, Tomas, Marcel, any ideas?
Comment 3 Jonathan Druart 2017-07-05 20:20:42 UTC
Ok, I am on it.
Comment 4 Jonathan Druart 2017-07-05 20:57:20 UTC
Created attachment 64830 [details] [review]
Bug 18851: Database handler must not be created at runtime
Comment 5 Jonathan Druart 2017-07-05 20:57:24 UTC
Created attachment 64831 [details] [review]
Bug 18851: Test::DBIx::Class is not needed in 00-load.t
Comment 6 Jonathan Druart 2017-07-05 20:57:28 UTC
Created attachment 64832 [details] [review]
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed
Comment 7 Jonathan Druart 2017-07-05 20:57:50 UTC
Mirko, could you confirm these patches fix the problem?
Comment 8 Jonathan Druart 2017-07-05 20:59:20 UTC
non-dependent tests must be tested with and without Test::DBIx::Class and with and without the DBMS running
Comment 9 Mirko Tietgen 2017-07-07 13:43:39 UTC
Problems with missing Test::DBIx::Class are still present in

t/Patron.t
t/Search.t
t/SuggestionEngine_AuthorityFile.t

Additionally there are a lot of tests failing because of missing DB. They should be skipped if DB is not available.
Comment 10 Mirko Tietgen 2017-07-07 13:45:22 UTC
Created attachment 64913 [details]
build log: tests

Attached file: build log: tests. Problems with Test::DBIx::Class and missing DB
Comment 11 Jonathan Druart 2017-07-07 16:32:44 UTC
I do not recreate the DB issues:

prove t/Biblio.t t/Biblio2.t
t/Biblio.t ... skipped: Need Test::DBIx::Class
t/Biblio2.t .. ok   
All tests successful.
Files=2, Tests=2,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.42 cusr  0.18 csys =  0.62 CPU)
Result: PASS

mysql is stopped and Test::DBIx::Class is not installed.
Comment 12 Jonathan Druart 2017-07-10 19:12:06 UTC
Created attachment 64979 [details] [review]
Bug 18851: Fix 3 other tests
Comment 13 Jonathan Druart 2017-07-10 19:56:43 UTC
I recreated the "Can't connect to MySQL server" errors, but actually they existed before bug 18266, there is nothing new here.
Comment 14 Mirko Tietgen 2017-07-11 07:47:50 UTC
(In reply to Jonathan Druart from comment #13)
> I recreated the "Can't connect to MySQL server" errors, but actually they
> existed before bug 18266, there is nothing new here.

Thanks. It must have been pushed around the same time then I guess. Last working nightly was June 5.
Comment 15 Mirko Tietgen 2017-07-11 10:09:24 UTC
Tests pass with the latest patch, nightly package + patch build works. I still get the DB messages, but they seem to be only warnings then. Thanks! I'll do a signoff when I get near a dev machine.
Comment 16 Mirko Tietgen 2017-07-11 10:58:04 UTC
Created attachment 64990 [details] [review]
Bug 18851: Database handler must not be created at runtime

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 17 Mirko Tietgen 2017-07-11 10:58:08 UTC
Created attachment 64991 [details] [review]
Bug 18851: Test::DBIx::Class is not needed in 00-load.t

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 18 Mirko Tietgen 2017-07-11 10:58:12 UTC
Created attachment 64992 [details] [review]
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 19 Mirko Tietgen 2017-07-11 10:58:16 UTC
Created attachment 64993 [details] [review]
Bug 18851: Fix 3 other tests

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 20 Nick Clemens 2017-07-21 10:21:15 UTC
These all work for me, however, t/Koha_ExternalContent_OverDrive.t fails for me without a db. Can you test with WebService::ILS::OverDrive::Patron installed and verify?
Comment 21 Jonathan Druart 2017-07-24 15:44:53 UTC
Created attachment 65199 [details] [review]
Bug 18851: Make OverDrive.T pass if dbms is not running
Comment 22 Nick Clemens 2017-07-31 10:34:07 UTC
With SQL stopped and no Test::DBIx::Class - all pass

With SQL stopped and Test::DBIx::Class:
t/Patron.t                        (Wstat: 65280 Tests: 2 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 11 tests but ran 2.
t/SuggestionEngine_AuthorityFile.t (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
  Parse errors: Bad plan.  You planned 46 tests but ran 4.
t/XSLT.t                          (Wstat: 65280 Tests: 8 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 9 tests but ran 8.

With SQL started and no Test::DBIx::Class - all pass

With SQL started and Test::DBIx::Class:
t/SuggestionEngine_AuthorityFile.t (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
  Parse errors: Bad plan.  You planned 46 tests but ran 4.
Comment 23 Jonathan Druart 2017-07-31 16:53:38 UTC
Created attachment 65368 [details] [review]
Bug 18851: Fix few other tests
Comment 24 Tomás Cohen Arazi 2017-08-07 15:36:20 UTC
Created attachment 65574 [details] [review]
Bug 18851: Database handler must not be created at runtime

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 25 Tomás Cohen Arazi 2017-08-07 15:36:26 UTC
Created attachment 65575 [details] [review]
Bug 18851: Test::DBIx::Class is not needed in 00-load.t

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 26 Tomás Cohen Arazi 2017-08-07 15:36:33 UTC
Created attachment 65576 [details] [review]
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 27 Tomás Cohen Arazi 2017-08-07 15:36:40 UTC
Created attachment 65577 [details] [review]
Bug 18851: Make OverDrive.T pass if dbms is not running

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 28 Tomás Cohen Arazi 2017-08-07 15:36:45 UTC
Created attachment 65578 [details] [review]
Bug 18851: Fix few other tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 29 Tomás Cohen Arazi 2017-08-07 15:36:51 UTC
Created attachment 65579 [details] [review]
Bug 18851: (QA followup) Test doesn't need sessions on DB

As the DB structure is not initialized for sessions, running this tests raise an irrelevant warning.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 30 Tomás Cohen Arazi 2017-08-07 15:38:08 UTC
I successfully ran all the tests:
- With and without a DB server running
- With and without Test::DBIx::Class installed

Added a tiny followup to avoid irrelevant warnings.
Comment 31 Jonathan Druart 2017-08-08 12:45:20 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 32 Fridolin Somers 2017-09-12 14:24:41 UTC
I have a lot of conflicts during rebase on 17.05.x and I'm not sure my test env is well configured to mock DB.
Comment 33 Mirko Tietgen 2018-06-15 15:46:35 UTC
Fridolin: I suppose you missed that it depends on bug 18226. I could easily apply both to 17.05.x, there is just one test to 'git add'. This would probably fix the failing t/XSLT.t in 17.05.x (with Test::DBiX::Class but without DB, as on the nightly server). Would you reconsider pushing to 17.05.x?
Comment 34 Mirko Tietgen 2018-06-15 16:07:31 UTC
Nevermind, it fails a lot more tests afterwards. There is more missing.
Comment 35 Mirko Tietgen 2018-06-15 16:24:33 UTC
Ok, it is, in this order,

18285
18286
18287
18288
18289
18226
18851

No rebase needed. All tests run fine afterwards (not checked db-dependent)
Comment 36 Fridolin Somers 2018-07-16 09:57:06 UTC
17.05.x end of life is in septembre so I will keep as it is