Bug 30125

Summary: Add full-stack tests for API pagination
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Test SuiteAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, kyle, martin.renvoize, nick
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00
Attachments: Bug 30125: Add full-stack tests for API pagination
Bug 30125: Add full-stack tests for API pagination
Bug 30125: (QA follow-up) Rename variable as cities
Bug 30125: (QA follow-up) Rename variable as cities

Description Tomás Cohen Arazi 2022-02-17 17:45:28 UTC
Currently we have some tests on the helper, and also a few (incomplete) tests in api/v1/checkouts.t

I propose we add a new file with full tests coverage for pagination, using the always-picked-as-sample GET /cities route.
Comment 1 Tomás Cohen Arazi 2022-02-17 18:56:34 UTC
Created attachment 130766 [details] [review]
Bug 30125: Add full-stack tests for API pagination

This patch adds tests for all the pagination use cases on a real route
(GET /cities). It tests _page, _per_page along with the returned Link
headers and total counts (i.e. X-Total-Count, X-Base-Total-Count), and
the results themselves.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/pagination.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Martin Renvoize 2022-02-18 11:56:04 UTC
Created attachment 130826 [details] [review]
Bug 30125: Add full-stack tests for API pagination

This patch adds tests for all the pagination use cases on a real route
(GET /cities). It tests _page, _per_page along with the returned Link
headers and total counts (i.e. X-Total-Count, X-Base-Total-Count), and
the results themselves.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/pagination.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2022-02-18 11:56:40 UTC
Excellent addition, clear code, tests passing.

Going straight for PQA.
Comment 4 Nick Clemens 2022-02-18 12:04:42 UTC
Created attachment 130827 [details] [review]
Bug 30125: (QA follow-up) Rename variable as cities

It's what we are testing :-)
Comment 5 Tomás Cohen Arazi 2022-02-18 12:06:51 UTC
(In reply to Nick Clemens from comment #4)
> Created attachment 130827 [details] [review] [review]
> Bug 30125: (QA follow-up) Rename variable as cities
> 
> It's what we are testing :-)

Can't you live with some bit of confusion dude? Haha
Comment 6 Fridolin Somers 2022-02-24 23:55:58 UTC
> is( scalar @{$cities}, 10, '10 libraries retrieved' );
Also rename '10 cities retrieved' no ?
Comment 7 Tomás Cohen Arazi 2022-02-25 08:18:43 UTC
Created attachment 131100 [details] [review]
Bug 30125: (QA follow-up) Rename variable as cities

It's what we are testing :-)
Comment 8 Tomás Cohen Arazi 2022-02-25 08:19:19 UTC
(In reply to Fridolin Somers from comment #6)
> > is( scalar @{$cities}, 10, '10 libraries retrieved' );
> Also rename '10 cities retrieved' no ?

Done. I've amended Nick's patch.
Comment 9 Fridolin Somers 2022-03-04 02:12:31 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄