[Bug 24229] New: /items API tests fail on Ubuntu 18.04
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Bug ID: 24229 Summary: /items API tests fail on Ubuntu 18.04 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Tests fail in a weird way: kohadev-koha@7369d4443675:/kohadevbox/koha$ prove t/db_dependent/api/v1/items.t t/db_dependent/api/v1/items.t .. # Inactivity timeout # Failed test 'GET //RTrSH14TLmPlTAsO1RAR9x8MhJtMO7XJS5IKOwBKfwcFnpzHSGSEPM7k:thePassword123@/api/v1/items' # at t/db_dependent/api/v1/items.t line 75. # Failed test 'SWAGGER3.2.2' # at t/db_dependent/api/v1/items.t line 75. # got: undef # expected: '200' # Looks like you planned 12 tests but ran 5. # Looks like you failed 2 tests of 5 run. t/db_dependent/api/v1/items.t .. 1/2 # Failed test 'list() tests' # at t/db_dependent/api/v1/items.t line 95. Can't use an undefined value as an ARRAY reference at t/db_dependent/api/v1/items.t line 79. # Looks like your test exited with 255 just after 1. t/db_dependent/api/v1/items.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 2/2 subtests It happens almost randomly (I reproduced the first time I started koha-testing-docker with the master-bionic image, but then it wouldn't reproduce). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |16825 Assignee|chris@bigballofwax.co.nz |tomascohen@gmail.com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16825 [Bug 16825] Add API route for getting an item -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Still, but without any info: koha_1 | [17:33:12] t/db_dependent/api/v1/items.t ........................................... koha_1 | Failed 2/2 subtests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 75 $t->get_ok( "//$userid:$password@/api/v1/items" ) 76 ->status_is( 200, 'SWAGGER3.2.2' ); This test is the slower when I run it locally, and You got "Inactivity timeout" when it failed. It there any kinds of timeout value we could extend? That would explain why it fails randomly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I do recreate the issue with running several time the tests: for i in {1..6}; do timeout 60 prove -v t/db_dependent/api/v1/items.t& done Make sure you have the timeout, or the whole thing will freeze! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I have tried to extend Mojo timeout https://mojolicious.org/perldoc/Mojolicious/Guides/FAQ#What-does-Inactivity-... export MOJO_INACTIVITY_TIMEOUT=120 for i in {1..6}; do timeout 120 prove -v t/db_dependent/api/v1/items.t& done I do no longer get the tests failing, but they all get stuck at this test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Not directly related, but I noticed that we return ALL the items from the DB, and it is what the tests expect: 75 $t->get_ok( "//$userid:$password@/api/v1/items" ) 76 ->status_is( 200, 'SWAGGER3.2.2' ); 77 78 my $items_count = Koha::Items->search->count; 79 my $response_count = scalar @{ $t->tx->res->json }; 80 81 is( $items_count, $response_count, 'The API returns all the items' ); Wondering if this is correct. If not please open a separate bug report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Tomas, I tried to remove the changes made by bug 24700 but it did not work. However, this worked (it is not a fix, but I wanted to confirm it came from there): diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index 81cc97633c..430518251c 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -116,6 +116,7 @@ sub register { $filtered_params = $c->merge_q_params( $filtered_params, $query_params, $result_set ); } # Perform search + $attributes->{page} = 1; my $objects = $result_set->search( $filtered_params, $attributes ); if ($objects->is_paged) { diff --git a/t/db_dependent/api/v1/items.t b/t/db_dependent/api/v1/items.t index 9eb623b96c..8b141ed781 100644 --- a/t/db_dependent/api/v1/items.t +++ b/t/db_dependent/api/v1/items.t @@ -78,7 +78,7 @@ subtest 'list() tests' => sub { my $items_count = Koha::Items->search->count; my $response_count = scalar @{ $t->tx->res->json }; - is( $items_count, $response_count, 'The API returns all the items' ); + is( 10, $response_count, 'The API returns all the items' ); $t->get_ok( "//$userid:$password@/api/v1/items?external_id=" . $item->barcode ) ->status_is(200) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25514 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25551 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25551 [Bug 25551] [OMNIBUS] Some tests are failing randomly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 105163 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105163&action=edit Bug 24229: Avoid fetching ALL items in test I'm not sure what we need to test here, besides we get results. I'm pretty sure we shouldn't allow a plain GET return all objects because it could cause a DOS very easily. But it certainly belongs to a separate bug/discussion. In this case, I put a constraint on the items domain (by using _per_page=10 in the query params) so it won't happen that under certain conditions the user agent used by Test::Mojo time outs. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #4)
I have tried to extend Mojo timeout https://mojolicious.org/perldoc/Mojolicious/Guides/FAQ#What-does-Inactivity- timeout-mean
export MOJO_INACTIVITY_TIMEOUT=120 for i in {1..6}; do timeout 120 prove -v t/db_dependent/api/v1/items.t& done
I do no longer get the tests failing, but they all get stuck at this test.
I reproduced the problem easily with your loop. My patch fixes that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #5)
Not directly related, but I noticed that we return ALL the items from the DB, and it is what the tests expect:
75 $t->get_ok( "//$userid:$password@/api/v1/items" ) 76 ->status_is( 200, 'SWAGGER3.2.2' ); 77 78 my $items_count = Koha::Items->search->count; 79 my $response_count = scalar @{ $t->tx->res->json }; 80 81 is( $items_count, $response_count, 'The API returns all the items' );
Wondering if this is correct. If not please open a separate bug report.
Tomas, can you answer this question please? As I said yesterday during the dev meeting, it would mean that one could call 6 times the GET /items route to kick the server down? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #9)
(In reply to Jonathan Druart from comment #5)
Not directly related, but I noticed that we return ALL the items from the DB, and it is what the tests expect:
75 $t->get_ok( "//$userid:$password@/api/v1/items" ) 76 ->status_is( 200, 'SWAGGER3.2.2' ); 77 78 my $items_count = Koha::Items->search->count; 79 my $response_count = scalar @{ $t->tx->res->json }; 80 81 is( $items_count, $response_count, 'The API returns all the items' );
Wondering if this is correct. If not please open a separate bug report.
Tomas, can you answer this question please?
As I said yesterday during the dev meeting, it would mean that one could call 6 times the GET /items route to kick the server down?
I think I did on my commit message, Jonathan. I'll rephrase. Test::Mojo creates a user agent that is used internally. This user agent has a low timeout. Loading the V1.pm app in Test::Mojo is not the same as loading it with Plack and I'm not sure about the internals. This is not different than running a badly written report that takes too much. It might keep a starman worker busy for too long, etc. I think we need to force pagination and at some point I was sure we did. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #10)
I think we need to force pagination and at some point I was sure we did.
Can you open a bug report for that please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105163|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105182 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105182&action=edit Bug 24229: Avoid fetching ALL items in test I'm not sure what we need to test here, besides we get results. I'm pretty sure we shouldn't allow a plain GET return all objects because it could cause a DOS very easily. But it certainly belongs to a separate bug/discussion. In this case, I put a constraint on the items domain (by using _per_page=10 in the query params) so it won't happen that under certain conditions the user agent used by Test::Mojo time outs. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105182|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 105193 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105193&action=edit Bug 24229: Avoid fetching ALL items in test I'm not sure what we need to test here, besides we get results. I'm pretty sure we shouldn't allow a plain GET return all objects because it could cause a DOS very easily. But it certainly belongs to a separate bug/discussion. In this case, I put a constraint on the items domain (by using _per_page=10 in the query params) so it won't happen that under certain conditions the user agent used by Test::Mojo time outs. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #11)
(In reply to Tomás Cohen Arazi from comment #10)
I think we need to force pagination and at some point I was sure we did.
Can you open a bug report for that please?
Tomas, Joubu said, I should tell you to file that bug ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25570 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25570 [Bug 25570] Listing requests should be paginated by default -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #14)
(In reply to Jonathan Druart from comment #11)
(In reply to Tomás Cohen Arazi from comment #10)
I think we need to force pagination and at some point I was sure we did.
Can you open a bug report for that please?
Tomas, Joubu said, I should tell you to file that bug ;)
Done, bug 25570. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Tomas, do we need this if we have bug 25570? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Keywords|rel_20_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |aleisha@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|20.05.00 |20.05.00, 19.11.07 released in| | --- Comment #18 from Aleisha Amohia <aleisha@catalyst.net.nz> --- backported to 19.11.x for 19.11.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24229 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #19 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Won't backport to 19.05.x, listing items didn't existing back then. https://gitlab.com/koha-community/Koha/-/blob/19.05.x/api/v1/swagger/paths/i... https://gitlab.com/koha-community/Koha/-/blob/19.11.x/api/v1/swagger/paths/i... Hence the tests failing with 404 errors ^^" -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org