Bug 30125 - Add full-stack tests for API pagination
Summary: Add full-stack tests for API pagination
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-17 17:45 UTC by Tomás Cohen Arazi
Modified: 2022-03-04 14:25 UTC (History)
5 users (show)

See Also:
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 (7.81 KB, patch)
2022-02-17 18:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30125: Add full-stack tests for API pagination (7.88 KB, patch)
2022-02-18 11:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30125: (QA follow-up) Rename variable as cities (4.49 KB, patch)
2022-02-18 12:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30125: (QA follow-up) Rename variable as cities (4.47 KB, patch)
2022-02-25 08: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 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 🦄