Bug 18807 - www/batch.t is failing
Summary: www/batch.t is failing
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16423 18806
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-14 19:55 UTC by Jonathan Druart
Modified: 2017-12-07 22:24 UTC (History)
6 users (show)

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


Attachments
Bug 18807: Fix tests www/batch.t (3.23 KB, patch)
2017-06-14 20:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18807: t/db_dependent/www/batch.t fails (921 bytes, patch)
2017-06-15 22:11 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18807: Fix tests www/batch.t (3.29 KB, patch)
2017-06-15 22:28 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 18807: t/db_dependent/www/batch.t fails (974 bytes, patch)
2017-06-15 22:28 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 18807: Fix tests www/batch.t (3.34 KB, patch)
2017-06-19 15:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18807: t/db_dependent/www/batch.t fails (1.16 KB, patch)
2017-06-19 15:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-06-14 19:55:12 UTC

    
Comment 1 Jonathan Druart 2017-06-14 19:57:08 UTC
Needs fix from bug 18806.
Comment 2 Jonathan Druart 2017-06-14 20:15:29 UTC
Created attachment 64299 [details] [review]
Bug 18807: Fix tests www/batch.t

Several things are wrong here:
1. It assumes that import_record_id is the biblionumber
=> Wrong, it is only true when the DB is empty and that the 2 AI equal 1
2. The encoding in the template is 'UTF-8', not 'utf8', it leaded to
  "stage-marc-import.pl: marc21record.mrc: Unexpected charset UTF-8, expecting utf8"
3. We did not test that the biblio was correctly imported

Test plan:
Make sure the tests now pass.
For the www tests you need to set the following env vars:
KOHA_USER, KOHA_PASS, KOHA_INTRANET_URL and KOHA_OPAC_URL
Comment 3 Jonathan Druart 2017-06-15 19:01:06 UTC
Block RM actions (cannot test before pushing stuffs).
Comment 4 M. Tompsett 2017-06-15 22:11:58 UTC
Created attachment 64364 [details] [review]
Bug 18807: t/db_dependent/www/batch.t fails

get() does not take two parameters. fixed.
prove and run koha qa test tools
Comment 5 Lee Jamison 2017-06-15 22:28:45 UTC
Created attachment 64369 [details] [review]
Bug 18807: Fix tests www/batch.t

Several things are wrong here:
1. It assumes that import_record_id is the biblionumber
=> Wrong, it is only true when the DB is empty and that the 2 AI equal 1
2. The encoding in the template is 'UTF-8', not 'utf8', it leaded to
  "stage-marc-import.pl: marc21record.mrc: Unexpected charset UTF-8, expecting utf8"
3. We did not test that the biblio was correctly imported

Test plan:
Make sure the tests now pass.
For the www tests you need to set the following env vars:
KOHA_USER, KOHA_PASS, KOHA_INTRANET_URL and KOHA_OPAC_URL

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 6 Lee Jamison 2017-06-15 22:28:55 UTC
Created attachment 64370 [details] [review]
Bug 18807: t/db_dependent/www/batch.t fails

get() does not take two parameters. fixed.
prove and run koha qa test tools

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 7 Marcel de Rooy 2017-06-16 08:26:17 UTC
Two questions:

Should this work under https too? At this moment I run into Error GETing https:<etc>/cgi-bin/koha/tools/stage-marc-import.pl: Forbidden at t/db_dependent/www/batch.t line 69.
Seeing reports of these errors with Mechanize and https when googling for it.

$agent->content_contains( 'Details for ' . $bookdescription, [etc]
You apparently assume here that the response is English from the staff client. Should you expect that? But note that it seems that the whole test is built on that assumption.. So just for the record.
Comment 8 Lee Jamison 2017-06-16 12:38:45 UTC
(In reply to Marcel de Rooy from comment #7)
> Two questions:
> 
> Should this work under https too? At this moment I run into Error GETing
> https:<etc>/cgi-bin/koha/tools/stage-marc-import.pl: Forbidden at
> t/db_dependent/www/batch.t line 69.
> Seeing reports of these errors with Mechanize and https when googling for it.
> 
> $agent->content_contains( 'Details for ' . $bookdescription, [etc]
> You apparently assume here that the response is English from the staff
> client. Should you expect that? But note that it seems that the whole test
> is built on that assumption.. So just for the record.

Hi Marcel, 

this may be ignorance on my part as I'm still getting familiarized with what the test suite offers but are there other tests/instances where this happens? Or is can this be considered an outlying occurrence?
Comment 9 Jonathan Druart 2017-06-16 13:55:30 UTC
(In reply to Marcel de Rooy from comment #7)
> Two questions:
> 
> Should this work under https too? At this moment I run into Error GETing
> https:<etc>/cgi-bin/koha/tools/stage-marc-import.pl: Forbidden at
> t/db_dependent/www/batch.t line 69.
> Seeing reports of these errors with Mechanize and https when googling for it.

Out of the scope of this bug report anyway. I guess other www tests do not pass either?

> $agent->content_contains( 'Details for ' . $bookdescription, [etc]
> You apparently assume here that the response is English from the staff
> client. Should you expect that? But note that it seems that the whole test
> is built on that assumption.. So just for the record.

Yes. Did you find another quick and easy solution?

The goal here is to get www tests I can run before pushing. So let's fix them first, then improve them later.
Comment 10 Marcel de Rooy 2017-06-19 13:20:46 UTC
(In reply to Jonathan Druart from comment #9)
> Out of the scope of this bug report anyway. I guess other www tests do not
> pass either?

Only history.t passes.

ok 4 - open tools module
Error GETing https:[etc]/cgi-bin/koha/tools/stage-marc-import.pl: Forbidden at www/search_utf8.t line 127.
Again stage marc import btw !?
Comment 11 Lee Jamison 2017-06-19 14:01:33 UTC
Prior to applying this patch, batch.t fails for me. After applying it, all t/db_dependent/www/ tests pass for me on kohadevbox
Comment 12 Tomás Cohen Arazi 2017-06-19 15:18:00 UTC
Created attachment 64429 [details] [review]
Bug 18807: Fix tests www/batch.t

Several things are wrong here:
1. It assumes that import_record_id is the biblionumber
=> Wrong, it is only true when the DB is empty and that the 2 AI equal 1
2. The encoding in the template is 'UTF-8', not 'utf8', it leaded to
  "stage-marc-import.pl: marc21record.mrc: Unexpected charset UTF-8, expecting utf8"
3. We did not test that the biblio was correctly imported

Test plan:
Make sure the tests now pass.
For the www tests you need to set the following env vars:
KOHA_USER, KOHA_PASS, KOHA_INTRANET_URL and KOHA_OPAC_URL

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2017-06-19 15:18:08 UTC
Created attachment 64430 [details] [review]
Bug 18807: t/db_dependent/www/batch.t fails

get() does not take two parameters. fixed.
prove and run koha qa test tools

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Jonathan Druart 2017-06-19 19:39:34 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 15 Fridolin SOMERS 2017-06-27 15:12:00 UTC
Pushed to 17.05.x, will be in 17.05.02

Awesome job guys ;)
Comment 16 Katrin Fischer 2017-06-27 22:09:20 UTC
Tests fail for me with and without this patch:
 
1..26
ok 1 - connect to intranet
ok 2 - login to staff client
ok 3 - load main page
ok 4 - open tools module
Link not found at t/db_dependent/www/batch.t line 69.
# Looks like your test exited with 255 just after 4.
Comment 17 Jonathan Druart 2017-07-05 23:17:09 UTC
(In reply to Katrin Fischer from comment #16)
> Tests fail for me with and without this patch:

master or 16.11.x?
Comment 18 Katrin Fischer 2017-07-06 06:08:39 UTC
Was testing with 16.11.x
Comment 19 Jonathan Druart 2017-07-06 15:11:53 UTC
(In reply to Katrin Fischer from comment #18)
> Was testing with 16.11.x

Tried just right now (HEAD is f5da355d4f2fc4435d1581afa0cbb462e9f389f0) and it works. Could you retry?
Comment 20 Katrin Fischer 2017-07-16 13:02:27 UTC
Tests all pass.

These patches have been pushed to 16.11.x and will be in 16.11.10.
Comment 21 Mason James 2017-07-31 11:31:17 UTC
Pushed to 16.05.x, for 16.05.15 release