Bug 9848 - SIP tests : fix in 10renew_all.t (additional checkin)
Summary: SIP tests : fix in 10renew_all.t (additional checkin)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-19 15:57 UTC by Adrien SAURAT
Modified: 2015-12-03 22:03 UTC (History)
6 users (show)

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


Attachments
proposed patch #1 (4.02 KB, patch)
2013-03-19 16:05 UTC, Adrien SAURAT
Details | Diff | Splinter Review
Bug 9848: SIP tests, fix in 10renew_all.t (additionnal checkin) (4.07 KB, patch)
2013-03-20 13:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9848: SIP tests, fix in 10renew_all.t (additionnal checkin) (4.21 KB, patch)
2014-12-24 11:18 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Adrien SAURAT 2013-03-19 15:57:24 UTC
[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.
Comment 1 Adrien SAURAT 2013-03-19 16:05:22 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2013-03-20 13:45:48 UTC Comment hidden (obsolete)
Comment 3 Mason James 2013-04-29 05:02:35 UTC
(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
Comment 4 Jonathan Druart 2014-12-24 11:18:04 UTC
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>
Comment 5 Jonathan Druart 2014-12-24 11:18:31 UTC
(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.
Comment 6 Katrin Fischer 2014-12-27 10:11:43 UTC
Jonathan, can you add some hints on how to run the tests?
Comment 7 Jonathan Druart 2015-01-05 11:24:01 UTC
(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.
Comment 8 Katrin Fischer 2015-01-21 07:22:04 UTC
Does noone know how to run these tests?
Comment 9 Colin Campbell 2015-01-21 10:25:47 UTC
(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
Comment 10 Katrin Fischer 2015-01-21 11:28:01 UTC
Ouch. What do you suggest for this bug? Should we push it to the 'reminder'?
Comment 11 Colin Campbell 2015-01-22 12:38:59 UTC
(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
Comment 12 Katrin Fischer 2015-03-03 11:34:06 UTC
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.
Comment 13 Tomás Cohen Arazi 2015-03-05 15:32:22 UTC
Patch pushed to master.

Thanks Adrien!
Comment 14 Chris Cormack 2015-03-12 07:16:49 UTC
Pushed to 3.18.x will be in 3.18.5