Bug 9209 - Mocked Koha::Calendar tests
Summary: Mocked Koha::Calendar tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Tomás Cohen Arazi
QA Contact: Paul Poulain
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 9211 11061
  Show dependency treegraph
 
Reported: 2012-12-04 16:01 UTC by Tomás Cohen Arazi
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 9209 - Mocked Koha::Calendar tests (17.49 KB, patch)
2012-12-04 18:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 9209 - Mocked Koha::Calendar tests (17.55 KB, patch)
2012-12-07 15:29 UTC, Elliott Davis
Details | Diff | Splinter Review
Bug 9209: Skip tests for older DBD::Mock (1010 bytes, patch)
2012-12-28 01:20 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9209: Skip tests for older DBD::Mock (1.05 KB, patch)
2012-12-28 01:54 UTC, Liz Rea
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2012-12-04 16:01:36 UTC
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).
Comment 1 Tomás Cohen Arazi 2012-12-04 18:28:40 UTC Comment hidden (obsolete)
Comment 2 Elliott Davis 2012-12-07 15:28:01 UTC
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
Comment 3 Elliott Davis 2012-12-07 15:29:51 UTC
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>
Comment 4 Jonathan Druart 2012-12-21 15:41:40 UTC
Are all tests supposed to pass with success ?
I get 11 test fails.
Comment 5 Tomás Cohen Arazi 2012-12-21 15:47:01 UTC
(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
Comment 6 Paul Poulain 2012-12-21 16:08:52 UTC
(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 ?
Comment 7 Paul Poulain 2012-12-21 16:09:19 UTC
(In reply to comment #6)
>  without manual configuration ?

s/without manual configuration/without manual cpan install/
Comment 8 Tomás Cohen Arazi 2012-12-21 16:59:54 UTC
(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.
Comment 9 Katrin Fischer 2012-12-27 21:47:27 UTC
Same for me - 1.43 and 11 tests failing.
Comment 10 Katrin Fischer 2012-12-27 22:36:09 UTC
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?
Comment 11 Jared Camins-Esakov 2012-12-28 01:20:28 UTC Comment hidden (obsolete)
Comment 12 Jared Camins-Esakov 2012-12-28 01:25:13 UTC
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.
Comment 13 Liz Rea 2012-12-28 01:54:10 UTC
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
Comment 14 Jared Camins-Esakov 2012-12-28 13:10:06 UTC
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.
Comment 15 Jared Camins-Esakov 2012-12-29 01:16:22 UTC
This patch has been pushed to master.
Comment 16 Chris Cormack 2013-01-01 18:24:43 UTC
Pushed to 3.10.x will be in 3.10.2