| Summary: | t/db_dependent/Reports_Guided.t should not depend on existing data | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | kyle, nick, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 15152: Try and make Guided Reports related tests pass
[SIGNED OFF] Bug 15152: Try and make Guided Reports related tests pass Bug 15152: t/db_dependent/Reports_Guided.t should not depend on existing data Bug 15152: t/db_dependent/Reports_Guided.t should not depend on existing data |
||
|
Description
Jonathan Druart
2015-11-06 15:12:11 UTC
Created attachment 44577 [details] [review] Bug 15152: Try and make Guided Reports related tests pass Jenkins is currently failing on t_db_dependent_Reports_Guided_t.filter takes report id (393) Failing for the past 1 build (Since Unstable#190 ) Took 2 ms. Error Message not ok 8 - filter takes report id Standard Output 1..19 ok 1 - use C4::Reports::Guided; ok 2 - Not breaking subqueries ok 3 - Returns correct default offset ok 4 - Returns correct default LIMIT ok 5 - Correctly removes only final LIMIT ok 6 - Returns correct default offset ok 7 - Returns correct extracted LIMIT ok 8 - Correctly removes only final LIMIT (with offset) ok 9 - Returns correct extracted offset ok 10 - Returns correct extracted LIMIT ok 11 - Not breaking simple queries ok 12 - Returns correct default offset ok 13 - Returns correct default LIMIT ok 14 - Correctly removes LIMIT in simple case ok 15 - Returns correct default offset ok 16 - Returns correct extracted LIMIT ok 17 - Correctly removes LIMIT in simple case (with offset) ok 18 - Returns correct extracted offset ok 19 - Returns correct extracted LIMIT Which does not make sense. I suspect that there is a mismatch with the different filenames. This patch groups the different tests for Guided reports into the same file. The goal is to make Jenkins happy going all-in with this assumption. Created attachment 44590 [details] [review] [SIGNED OFF] Bug 15152: Try and make Guided Reports related tests pass Jenkins is currently failing on t_db_dependent_Reports_Guided_t.filter takes report id (393) Failing for the past 1 build (Since Unstable#190 ) Took 2 ms. Error Message not ok 8 - filter takes report id Standard Output 1..19 ok 1 - use C4::Reports::Guided; ok 2 - Not breaking subqueries ok 3 - Returns correct default offset ok 4 - Returns correct default LIMIT ok 5 - Correctly removes only final LIMIT ok 6 - Returns correct default offset ok 7 - Returns correct extracted LIMIT ok 8 - Correctly removes only final LIMIT (with offset) ok 9 - Returns correct extracted offset ok 10 - Returns correct extracted LIMIT ok 11 - Not breaking simple queries ok 12 - Returns correct default offset ok 13 - Returns correct default LIMIT ok 14 - Correctly removes LIMIT in simple case ok 15 - Returns correct default offset ok 16 - Returns correct extracted LIMIT ok 17 - Correctly removes LIMIT in simple case (with offset) ok 18 - Returns correct extracted offset ok 19 - Returns correct extracted LIMIT Which does not make sense. I suspect that there is a mismatch with the different filenames. This patch groups the different tests for Guided reports into the same file. The goal is to make Jenkins happy going all-in with this assumption. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Test plan used: 1 - prove t/db_dependent/Reports_Guided.t - Fail test 8 2 - apply patches 3 - prove -v t/db_dependent/Reports/Guided.t 4 - all tests pass (In reply to Nick Clemens from comment #3) > Test plan used: > 1 - prove t/db_dependent/Reports_Guided.t - Fail test 8 Oh, really? What is the error? Well...I got it before, but can no longer reproduce Interestingly, the test did fail the first time for me as well, and always after that: prove t/db_dependent/Reports_Guided.t t/db_dependent/Reports_Guided.t .. 3/18 # Failed test 'filter takes report id' # at t/db_dependent/Reports_Guided.t line 70. # got: '2' # expected: '1' # Looks like you failed 1 test of 18. Test Summary Report ------------------- t/db_dependent/Reports_Guided.t (Wstat: 256 Tests: 18 Failed: 1) Failed test: 8 Non-zero exit status: 1 Files=1, Tests=18, 2 wallclock secs ( 0.03 usr 0.00 sys + 1.09 cusr 0.05 csys = 1.17 CPU) Result: FAIL That should have read: ... and passed always after that. (In reply to Katrin Fischer from comment #7) > That should have read: ... and passed always after that. Yeah, jenkins loads a fresh 3.20.0 db each time, that's why it always fails. Can we have this tests split so non-db_dependent tests are in t/? I need someone to send me a DB (or a way to create it) where I can reproduce the failure. (In reply to Jonathan Druart from comment #10) > I need someone to send me a DB (or a way to create it) where I can reproduce > the failure. Here you go: [To set the failure scenario] $ vagrant up jessie $ vagrant ssh jessie $ sudo -s $ mysql > DROP DATABASE koha_kohadev; CREATE DATABASE koha_kohadev; \q $ zcat /vagrant/koha_3_20_00.sql.gz | mysql koha_kohadev $ exit $ sudo koha-shell kohadev $ cd kohaclone $ cat /vagrant/reports_guided_sequence.txt | xargs prove now run the tests: $ prove t/db_dependent/Reports_Guided.t => FAIL: The tests fail like this: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Reports_Guided.t t/db_dependent/Reports_Guided.t .. 3/18 # Failed test 'filter takes report id' # at t/db_dependent/Reports_Guided.t line 70. # got: '2' # expected: '1' # Looks like you failed 1 test of 18. t/db_dependent/Reports_Guided.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/18 subtests Test Summary Report ------------------- t/db_dependent/Reports_Guided.t (Wstat: 256 Tests: 18 Failed: 1) Failed test: 8 Non-zero exit status: 1 Files=1, Tests=18, 1 wallclock secs ( 0.03 usr 0.01 sys + 0.86 cusr 0.39 csys = 1.29 CPU) Result: FAIL run them again => SUCCESS: They now pass (i.e. side effects). (In reply to Tomás Cohen Arazi from comment #11) > (In reply to Jonathan Druart from comment #10) > > I need someone to send me a DB (or a way to create it) where I can reproduce > > the failure. I forgot the updatedatabase.pl step: $ sudo koha-shell kohadev $ cd kohaclone $ perl installer/data/mysql/updatedatabase.pl Also, the needed file can be grabbed like this: (on your kohadevbox dir) (kohadevbox)$ wget https://theke.io/static/reports_guided_sequence.txt (kohadevbox)$ wget https://theke.io/static/koha_3_20_00.sql.gz Created attachment 44881 [details] [review] Bug 15152: t/db_dependent/Reports_Guided.t should not depend on existing data The current implementation relies on existing borrowers on the DB. Even when the DB itself doesn't have a FK constraint, the function that retrieves the reports from the DB relies on get_saved_reports_base_query which creates a query that JOINs the full query with the 'borrowers' table, on the borrowernumber. So it is doomed to fail on some scenarios. Specially because the current tests expect specific borrowernumber values to be present (1, 2 and 3). To test: [To set the failure scenario] $ vagrant up jessie $ vagrant ssh jessie $ wget https://theke.io/static/reports_guided_sequence.txt -O /vagrant/reports_guided_sequence.txt $ wget https://theke.io/static/koha_3_20_00.sql.gz -O /vagrant/koha_3_20_00.sql.gz $ sudo -s $ mysql > DROP DATABASE koha_kohadev; CREATE DATABASE koha_kohadev; \q $ zcat /vagrant/koha_3_20_00.sql.gz | mysql koha_kohadev $ exit $ sudo koha-shell kohadev $ cd kohaclone $ cat /vagrant/reports_guided_sequence.txt | xargs prove - now run the tests: $ prove t/db_dependent/Reports_Guided.t => FAIL: The tests fail - Apply the patch - [re create the scenario] - Run the test: $ prove t/db_dependent/Reports_Guided.t => SUCCESS: Tests now pass - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 44884 [details] [review] Bug 15152: t/db_dependent/Reports_Guided.t should not depend on existing data The current implementation relies on existing borrowers on the DB. Even when the DB itself doesn't have a FK constraint, the function that retrieves the reports from the DB relies on get_saved_reports_base_query which creates a query that JOINs the full query with the 'borrowers' table, on the borrowernumber. So it is doomed to fail on some scenarios. Specially because the current tests expect specific borrowernumber values to be present (1, 2 and 3). To test: [To set the failure scenario] $ vagrant up jessie $ vagrant ssh jessie $ wget https://theke.io/static/reports_guided_sequence.txt -O /vagrant/reports_guided_sequence.txt $ wget https://theke.io/static/koha_3_20_00.sql.gz -O /vagrant/koha_3_20_00.sql.gz $ sudo -s $ mysql > DROP DATABASE koha_kohadev; CREATE DATABASE koha_kohadev; \q $ zcat /vagrant/koha_3_20_00.sql.gz | mysql koha_kohadev $ exit $ sudo koha-shell kohadev $ cd kohaclone $ cat /vagrant/reports_guided_sequence.txt | xargs prove - now run the tests: $ prove t/db_dependent/Reports_Guided.t => FAIL: The tests fail - Apply the patch - [re create the scenario] - Run the test: $ prove t/db_dependent/Reports_Guided.t => SUCCESS: Tests now pass - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Patch pushed to master. |