Using specific method for populating the internal data structures from Koha::Calendar has yielded to the non-detection of several bugs. There are also several tests that where db_dependent which is not always desirable. I propose the use of DBD::Mock (::Session) for using the actual code used by Koha in production for testing, mocking the DB queries itselves. I also took the time to repeat several tests in different syspref configurations (they applied only to daysMode=Calendar, and now cover all confs).
Created attachment 13885 [details] [review] Bug 9209 - Mocked Koha::Calendar tests Using specific method for populating the internal data structures from Koha::Calendar has yielded to the non-detection of several bugs. There are also several tests that where db_dependent which is not always desirable. I propose the use of DBD::Mock (::Session) for using the actual code used by Koha in production for testing, mocking the DB queries itselves. I also took the time to repeat several tests in different syspref configurations (they applied only to daysMode=Calendar, and now cover all confs). Notes: - I used DBD:Mock 1.45 as previous version (1.43, from 12.04) was broken - Some tests revealed a bug on days_between as I see it... reporting as Bug #9211 Sponsored-by: Universidad Nacional de Córdoba
Looks like the parameter flip isn't returning the desired behavior. I believe you have addressed the bug in 9211. t/Calendar.t .. 1/35 # Failed test 'Test parameter order not relevant (Days)' # at t/Calendar.t line 240. # got: 56 # expected: 40 # Failed test 'Test parameter order not relevant (Calendar)' # at t/Calendar.t line 280. # got: 56 # expected: 40 # Failed test 'Test parameter order not relevant (Days)' # at t/Calendar.t line 319. # got: 56 # expected: 40 # Looks like you failed 3 tests of 35. t/Calendar.t .. Dubious, test returned 3 (wstat 768, 0x300) Failed 3/35 subtests Test Summary Report ------------------- t/Calendar.t (Wstat: 768 Tests: 35 Failed: 3) Failed tests: 23, 29, 35 Non-zero exit status: 3 Files=1, Tests=35, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.37 cusr 0.01 csys = 0.40 CPU) Result: FAIL
Created attachment 13932 [details] [review] Bug 9209 - Mocked Koha::Calendar tests Using specific method for populating the internal data structures from Koha::Calendar has yielded to the non-detection of several bugs. There are also several tests that where db_dependent which is not always desirable. I propose the use of DBD::Mock (::Session) for using the actual code used by Koha in production for testing, mocking the DB queries itselves. I also took the time to repeat several tests in different syspref configurations (they applied only to daysMode=Calendar, and now cover all confs). Notes: - I used DBD:Mock 1.45 as previous version (1.43, from 12.04) was broken - Some tests revealed a bug on days_between as I see it... reporting as Bug #9211 Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Elliott Davis <elliott@bywatersolutions.com>
Are all tests supposed to pass with success ? I get 11 test fails.
(In reply to comment #4) > Are all tests supposed to pass with success ? > I get 11 test fails. Please check you have the latest DBD::Mock via CPAN as 1.43 had issues. There should be three tests failing, addressed by 9211
(In reply to comment #5) > (In reply to comment #4) > > Are all tests supposed to pass with success ? > > I get 11 test fails. > > Please check you have the latest DBD::Mock via CPAN as 1.43 had issues. > There should be three tests failing, addressed by 9211 Which issues have you seen ? DBD::Mock was used until now without any problem, unless i'm wrong. So more details are required before I can QA this patch (i've tested it and confirm i've 11 tests failing on my ubuntu 12.04, as joubu had) is it working on a debian stable without manual configuration ?
(In reply to comment #6) > without manual configuration ? s/without manual configuration/without manual cpan install/
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Are all tests supposed to pass with success ? > > > I get 11 test fails. > > > > Please check you have the latest DBD::Mock via CPAN as 1.43 had issues. > > There should be three tests failing, addressed by 9211 > > Which issues have you seen ? DBD::Mock was used until now without any > problem, unless i'm wrong. You are not wrong, the problem is related to DBD::Mock::Session and fetchrow_hashref together I guess. https://rt.cpan.org/Public/Bug/Display.html?id=71438 Please check which version of DBD::Mock are you using. If it is 1.43, then maybe that's the problem.
Same for me - 1.43 and 11 tests failing.
Not sure how to fix this - maybe we could comment out the tests that are broken now and reactivate them later when a new version is available?
Created attachment 14305 [details] [review] Bug 9209: Skip tests for older DBD::Mock Because the calendar tests require a more recent version of DBD::Mock than has been packaged by Debian, it makes sense to skip all the tests requiring the database when only an older version is present.
The test plan for the follow-up is the same as for the original: run `prove t/Calendar.t`. If you have a version of DBD::Mock older than 1.45, all the tests will be skipped once the follow-up is applied.
Created attachment 14306 [details] [review] Bug 9209: Skip tests for older DBD::Mock Because the calendar tests require a more recent version of DBD::Mock than has been packaged by Debian, it makes sense to skip all the tests requiring the database when only an older version is present. Signed-off-by: Liz Rea <liz@catalyst.net.nz> Works as advertised
Although I would not usually do this, since QAing these patches presents a problem for the QA team, and I am able to, I am going to go ahead and mark these patches as Passed QA. The test is now a no-op for systems that do not have DBD::Mock installed from CPAN and so are safe, and without them we will have no way to spot if further work on the calendar code breaks things.
This patch has been pushed to master.
Pushed to 3.10.x will be in 3.10.2