The unit tests t/db_dependent/Circulation/Returns.t and t/db_dependent/Items.t print out eh warning "item-level_itypes set but no itemtype set for item". This should not happen.
Created attachment 46869 [details] [review] Bug 15599 - Unit tests output warnings The unit tests t/db_dependent/Circulation/Returns.t and t/db_dependent/Items.t print out eh warning "item-level_itypes set but no itemtype set for item". This should not happen. Test Plan: 1) prove t/db_dependent/Circulation/Returns.t 2) Note the warning 3) prove t/db_dependent/Items.t 4) Note the warning 5) Apply this patch 6) prove t/db_dependent/Circulation/Returns.t 7) Note there is no warning 8) prove t/db_dependent/Items.t 9) Note there is no warning
Tried to test this and still generated errors. Not sure if it was due to me applying the process wrong or if patch is unusable.
(In reply to Gus Ellerm from comment #2) > Tried to test this and still generated errors. > Not sure if it was due to me applying the process wrong or if patch is > unusable. Everything still works for me. Did you apply bug 14598 first? If so, can you paste the output of the unit tests here?
Results for first prove - without patches installed kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Circulation/Returns.t t/db_dependent/Circulation/Returns.t .. Subroutine C4::Context::userenv redefined at t/db_dependent/Circulation/Returns.t line 15. t/db_dependent/Circulation/Returns.t .. ok All tests successful. Files=1, Tests=2, 1 wallclock secs ( 0.03 usr 0.00 sys + 0.98 cusr 0.09 csys = 1.10 CPU) Result: PASS Results for second prove - without patches installed t/db_dependent/Items.t .. 7/9 # Failed test 'Got Koha::Library from home_branch method' # at t/db_dependent/Items.t line 456. # got: 'Koha::Branch' # expected: 'Koha::Library' # Failed test 'Got Koha::Library from holding_branch method' # at t/db_dependent/Items.t line 460. # got: 'Koha::Branch' # expected: 'Koha::Library' # Looks like you failed 2 tests of 5. # Failed test 'Koha::Item(s) tests' # at t/db_dependent/Items.t line 464. # Looks like you failed 1 test of 9. t/db_dependent/Items.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/9 subtests Test Summary Report ------------------- t/db_dependent/Items.t (Wstat: 256 Tests: 9 Failed: 1) Failed test: 8 Non-zero exit status: 1 Files=1, Tests=9, 2 wallclock secs ( 0.03 usr 0.03 sys + 1.04 cusr 0.19 csys = 1.29 CPU) Result: FAIL Looks like I am missing some sample data. These tests were all done before application off patches.
Can't duplicate problem as described in test plan. mtompset@debian:~/kohaclone$ git checkout -b bug_15599 origin/master Branch bug_15599 set up to track remote branch master from origin. Switched to a new branch 'bug_15599' mtompset@debian:~/kohaclone$ prove t/db_dependent/Circulation/Returns.t t/db_dependent/Circulation/Returns.t .. Subroutine C4::Context::userenv redefined at t/db_dependent/Circulation/Returns.t line 15. t/db_dependent/Circulation/Returns.t .. ok All tests successful. Files=1, Tests=2, 1 wallclock secs ( 0.02 usr 0.00 sys + 0.90 cusr 0.04 csys = 0.96 CPU) Result: PASS mtompset@debian:~/kohaclone$ prove t/db_dependent/Items.t t/db_dependent/Items.t .. ok All tests successful. Files=1, Tests=9, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.16 cusr 0.03 csys = 1.21 CPU) Result: PASS Patch does not apply. mtompset@debian:~/kohaclone$ git bz apply 15599 Bug 15599 - Unit tests output warnings 46869 - Bug 15599 - Unit tests output warnings Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 15599 - Unit tests output warnings fatal: sha1 information is lacking or useless (t/db_dependent/Circulation/Returns.t). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 15599 - Unit tests output warnings The copy of the patch that failed is found in: /home/mtompset/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-15599---Unit-tests-output-warnings-lEGWsI.patch mtompset@debian:~/kohaclone$ NOTE: I read the patch, and it looks like an improvement to me, but can't sign off. Please rebase first.
Created attachment 47243 [details] [review] Bug 15599 - Unit tests output warnings The unit tests t/db_dependent/Circulation/Returns.t and t/db_dependent/Items.t print out eh warning "item-level_itypes set but no itemtype set for item". This should not happen. Test Plan: 1) prove t/db_dependent/Circulation/Returns.t 2) Note the warning 3) prove t/db_dependent/Items.t 4) Note the warning 5) Apply this patch 6) prove t/db_dependent/Circulation/Returns.t 7) Note there is no warning 8) prove t/db_dependent/Items.t 9) Note there is no warning Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Kyle, the warnings appear for many other tests, see t/db_dependent/Reserves.t for instance. I don't think this is a right way to fix it.
(In reply to Jonathan Druart from comment #7) > Kyle, the warnings appear for many other tests, see > t/db_dependent/Reserves.t for instance. > I don't think this is a right way to fix it. Perhaps not overall, however, taking "string $foo->bar" and turning it to "string $foobar" is a start. Catching warnings is a good test. Perhaps "$debug and" prepended to the {warn mess} would be fitting, and still allow for the testing added?
(In reply to M. Tompsett from comment #8) > (In reply to Jonathan Druart from comment #7) > > Kyle, the warnings appear for many other tests, see > > t/db_dependent/Reserves.t for instance. > > I don't think this is a right way to fix it. > > Perhaps not overall, however, taking "string $foo->bar" and turning it to > "string $foobar" is a start. Catching warnings is a good test. Perhaps > "$debug and" prepended to the {warn mess} would be fitting, and still allow > for the testing added? I'm open to whatever solution may be proposed. This is starting to go out of scope. The bug is for just two unit tests. I think the $debug and warn idea sounds good! We could have it set to 1 by default and set it to 0 in our unit tests.
I don't agree with Jonathan on this patch. I think tests (like Reserves.t) should create well-formed scenarios. If they are raising warnings, then they need to be fixed. And that's not the situation here. because we are explicitly creating this 'bad' scenario, to test the behaviour in that specific scenario. So, worth testing for the warning this time. I might be missing something anyway, so I'm open to discuss this. It doesn't apply BTW, because of the latest rebase of bug 14598.
There are currently a lot of itemtype things going on right now. These two test cases are rather quiet currently.
Created attachment 49973 [details] [review] Bug 15599: Unit tests output warnings The unit tests t/db_dependent/Circulation/Returns.t and t/db_dependent/Items.t print out eh warning "item-level_itypes set but no itemtype set for item". This should not happen. Test Plan: 1) prove t/db_dependent/Circulation/Returns.t 2) Note the warning 3) prove t/db_dependent/Items.t 4) Note the warning 5) Apply this patch 6) prove t/db_dependent/Circulation/Returns.t 7) Note there is no warning 8) prove t/db_dependent/Items.t 9) Note there is no warning Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar> Meaningful chages to the tests. Passes qa tests.
I pretty sure this patch is no longer necessary.