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.
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>
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>
Excellent addition, clear code, tests passing. Going straight for PQA.
Created attachment 130827 [details] [review] Bug 30125: (QA follow-up) Rename variable as cities It's what we are testing :-)
(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
> is( scalar @{$cities}, 10, '10 libraries retrieved' ); Also rename '10 cities retrieved' no ?
Created attachment 131100 [details] [review] Bug 30125: (QA follow-up) Rename variable as cities It's what we are testing :-)
(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.
Pushed to master for 22.05, thanks to everybody involved [U+1F984]