Bug 7661 - Moving C4::tests to unit tests
Summary: Moving C4::tests to unit tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low trivial (vote)
Assignee: Marcel de Rooy
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-07 14:42 UTC by Marcel de Rooy
Modified: 2013-03-21 13:03 UTC (History)
2 users (show)

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


Attachments
Patch (31.36 KB, patch)
2012-03-07 14:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
7661 Moving C4::tests to unit tests (4.33 KB, patch)
2012-03-08 02:57 UTC, Liz Rea
Details | Diff | Splinter Review
Followup patch (3.86 KB, patch)
2012-03-14 12:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2012-03-07 14:42:41 UTC
C4::tests just includes a C4::Record unit test.
I am moving it to t/db_dependent/Record.
Note: When I tested this, it fails tests 4 and 7 (2 from 23). This is already the case in master now, so outside the scope of this report.
Comment 1 Marcel de Rooy 2012-03-07 14:45:55 UTC Comment hidden (obsolete)
Comment 2 Liz Rea 2012-03-08 02:57:26 UTC Comment hidden (obsolete)
Comment 3 Ian Walls 2012-03-08 17:31:17 UTC
I believe t/db_dependent/Record/Record.t is redundant; there is a nearly identical file in t/db_dependent with the same name.
Comment 4 Marcel de Rooy 2012-03-09 09:23:35 UTC
(In reply to comment #3)
> I believe t/db_dependent/Record/Record.t is redundant; there is a nearly
> identical file in t/db_dependent with the same name.

I just had a quick glance through that file and chose the careful path: move the work of someone else to a more appropriate location instead of deleting it rightaway.
You are of course welcome to dig deeper and get the best of both in one test. Please send a followup ;)
Comment 5 Ian Walls 2012-03-09 15:51:38 UTC
Fair enough.  This patch just moves files, no regressions found.  Cleanup for tests can be accomplished in a follow up.  Marking as Passed QA.
Comment 6 Paul Poulain 2012-03-12 12:50:12 UTC
mmm... isn't the 31.36KB patch (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8055) really obsoleted ? I think it's a mistake, could you confirm ?
Comment 7 Marcel de Rooy 2012-03-12 12:52:54 UTC
(In reply to comment #6)
> mmm... isn't the 31.36KB patch
> (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8055) really
> obsoleted ? I think it's a mistake, could you confirm ?

Think it's okay. Obsoleted by signoff. The newer git version creates a smaller patch file.
Comment 8 Paul Poulain 2012-03-13 17:13:33 UTC
trying to prove t/db_dependent/Record/Record.t after applying the patch, I get a nasty:

t/db_dependent/Record/Record.t .. 1/23 Aucun fichier ou dossier de ce type at t/db_dependent/Record/Record.t line 46.
# Looks like you planned 23 tests but ran 1.
# Looks like your test exited with 2 just after 1.
t/db_dependent/Record/Record.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 22/23 subtests 

Test Summary Report
-------------------
t/db_dependent/Record/Record.t (Wstat: 512 Tests: 1 Failed: 0)
  Non-zero exit status: 2
  Parse errors: Bad plan.  You planned 23 tests but ran 1.
Files=1, Tests=1,  1 wallclock secs ( 0.04 usr  0.01 sys +  0.37 cusr  0.03 csys =  0.45 CPU)
Result: FAIL


What's wrong ?
I also had a problem applying the patch:
git bz apply 7661
Bug 7661 - Moving C4::tests to unit tests

7661 Moving C4::tests to unit tests
Apply? [yn] y

Applying: 7661 Moving C4::tests to unit tests
Can't open perl script "C4/tests/Record_test.pl": Aucun fichier ou dossier de ce type

Are you sure you want to commit ?
You can commit with the --no-verify argument
Patch left in /tmp/7661-Moving-C4tests-to-unit-tests-EEw6sJ.patch


that was caused by the pre-apply hook I think, I had to force --no-verify

I'll push it, we will see if things are OK on jenkins, but I have some doubts.
Comment 9 Marcel de Rooy 2012-03-14 12:38:24 UTC
Will send a followup shortly. Tested it in its Record subdirectory. From the root it does not work.
Comment 10 Marcel de Rooy 2012-03-14 12:46:16 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2012-03-14 12:47:12 UTC
Back to Passed QA for trivial followup resolving path and commenting out the two failing tests.
Comment 12 Paul Poulain 2012-03-14 14:01:55 UTC
follow-up pushed. The tests are now OK for me.