[MT10401] An error occurs while trying the "renew all" SIP test, 10renew_all.t: # Failed test 'match leader Renew All: prep: check out 1783B to 1X981' # at /home/asaurat/workspace/versions/community/C4/SIP/t/SIPtest.pm line 204. # Response '121YNY20130319 153138AOVIL|AA1X981|AB1783B|AJL ANGOISSE DU ROI SALOMON|AH20130521 235900|AFItem already checked out to you: renewing item.|AY3AZD551' doesn't match pattern '(?-xism:^121NNY\d{8} {4}\d{6})' Koha sent: 121YNY The test expects: 121NNY 12 = code for "Checkout Response" message 1 = Operation succeeded The three following characters mean: - renewal ok - magnetic data - desensitize We have a difference on the "Renewal ok" field. I found the reason behind this: it happens when you run all the tests at once ("make test"). In this case, this "renew all" test starts with an item already checked out during a former test. The "renew all" test begins with a checkout operation, but as the item is already out, this results in a renewal when it's not expected yet. The solution I can see : adding a "checkin" operation at the very beginning of the test. But to let the test work even when launched alone, we must allow this checkin to be unsuccessfull, it's just part of the preparation before the actual renewal tests.
Created attachment 16418 [details] [review] proposed patch #1 Adds the new checkin operations explained above. + some cleaning + moved a block to enhance readibility (keeping execution order)
Created attachment 16517 [details] [review] Bug 9848: SIP tests, fix in 10renew_all.t (additionnal checkin) Adds a checkin operation at the beginning of the test. Otherwise, a former test leaves the test item checked out and this generates an error. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to comment #2) > Created attachment 16517 [details] [review] [review] > Bug 9848: SIP tests, fix in 10renew_all.t (additionnal checkin) > > Adds a checkin operation at the beginning of the test. > Otherwise, a former test leaves the test item checked out > and this generates an error. > > Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> qa-comment: sorry, i spent a long time attempting to configure a sip-server to test this patch ...but was unsuccessful :/ someone else with a working sip will have to QA this Adrien, can you please perltidy this file, so qa-tool will pass it? http://wiki.koha-community.org/wiki/Coding_Guidelines#PERL1:_Perltidy
Created attachment 34676 [details] [review] Bug 9848: SIP tests, fix in 10renew_all.t (additionnal checkin) Adds a checkin operation at the beginning of the test. Otherwise, a former test leaves the test item checked out and this generates an error. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to Mason James from comment #3) > Adrien, can you please perltidy this file, so qa-tool will pass it? > http://wiki.koha-community.org/wiki/Coding_Guidelines#PERL1:_Perltidy The changes have been perltidied.
Jonathan, can you add some hints on how to run the tests?
(In reply to Katrin Fischer from comment #6) > Jonathan, can you add some hints on how to run the tests? I don't know and I did not find any documentation on these tests.
Does noone know how to run these tests?
(In reply to Katrin Fischer from comment #8) > Does noone know how to run these tests? The only documentation is the README in the directory. I took a look ages ago but a lot of the tests have hardcoded references to a test setup on a Liblime test system. As a whole they need rewitten as part of the test suite and to handle dependencies in a reproduceable way. They arnt a test suite they are a reminder that one needs to be written
Ouch. What do you suggest for this bug? Should we push it to the 'reminder'?
(In reply to Katrin Fischer from comment #10) > Ouch. What do you suggest for this bug? Should we push it to the 'reminder'? The patch makes logical sense. The problem is as it appears noone runs these tests regularly, noone is confident about verifying it. No problem with it, we must assume Adrien did run these to find the problem
I am in favor of pushing these - what I get from the discussion is, that it 'looks right', so it might help when the old testing files are ever used for reference in writing new tests.
Patch pushed to master. Thanks Adrien!
Pushed to 3.18.x will be in 3.18.5