Bugzilla – Attachment 181869 Details for
Bug 39817
All list endpoints should have the x-koha-request-id parameter definition
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 39817: Add some GET routes tests
Bug-39817-Add-some-GET-routes-tests.patch (text/plain), 4.72 KB, created by
Tomás Cohen Arazi (tcohen)
on 2025-05-02 14:16:16 UTC
(
hide
)
Description:
Bug 39817: Add some GET routes tests
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2025-05-02 14:16:16 UTC
Size:
4.72 KB
patch
obsolete
>From c91d3f2fbae6ccdbcf01aab0b62a559ea7b3b7c4 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Fri, 2 May 2025 11:11:02 -0300 >Subject: [PATCH] Bug 39817: Add some GET routes tests > >This patch adds tests for to-be mandatory parameters for plural >endpoints: > >* match >* order_by >* page >* per_page >* q_param >* q_body >* request_id_header > >Exceptions should be added if found in the wild, but the tests are >needed. > >Another check it adds is GET requests `operationId` should start with: > >* list >* get > >To test: >1. Run: > $ ktd --shell > k$ prove xt/api.t >=> FAIL: Tests pass! But our spec is bad >2. Apply this patch >3. Repeat 1 >=> SUCCESS: All mistakes make the tests fail! >4. Sign off :-D > >Note: a follow-up patch/set of patches should make this tests pass! > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > xt/api.t | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 75 insertions(+), 1 deletion(-) > >diff --git a/xt/api.t b/xt/api.t >index 78cf588c245..7e77a16e86b 100755 >--- a/xt/api.t >+++ b/xt/api.t >@@ -21,7 +21,7 @@ use Data::Dumper; > > use FindBin(); > use IPC::Cmd qw(can_run); >-use List::MoreUtils qw(any); >+use List::MoreUtils qw(any none); > use File::Slurp qw(read_file); > > my $t = Test::Mojo->new('Koha::REST::V1'); >@@ -172,6 +172,80 @@ subtest '400 response tests' => sub { > } > }; > >+subtest 'GET routes parameters tests' => sub { >+ >+ plan tests => 1; >+ >+ my @errors; >+ >+ foreach my $route ( sort keys %{$paths} ) { >+ foreach my $verb ( keys %{ $paths->{$route} } ) { >+ >+ next unless $verb eq 'get'; >+ >+ if ( $paths->{$route}->{$verb}->{operationId} !~ m/^list/ >+ && $paths->{$route}->{$verb}->{operationId} !~ m/^get/ ) >+ { >+ push @errors, >+ sprintf( >+ "%s %s -> bad operationId: %s (must start with list or get)", $verb, $route, >+ $paths->{$route}->{$verb}->{operationId} >+ ); >+ next; >+ } >+ >+ my @parameters = @{ $paths->{$route}->{$verb}->{parameters} }; >+ >+ my $plural = ( any { $_->{in} eq 'body' && $_->{name} eq 'query' } @parameters ) >+ || ( any { $_->{in} eq 'query' && $_->{name} eq 'q' } @parameters ); >+ >+ if ($plural) { >+ >+ # 'q' as query parameter needs to be supported >+ if ( none { $_->{in} eq 'query' && $_->{name} eq 'q' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing 'q' parameter", $verb, $route ); >+ } >+ >+ # 'query' in body needs to be supported >+ if ( none { $_->{in} eq 'body' && $_->{name} eq 'query' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing 'query' parameter", $verb, $route ); >+ } >+ >+ # plural lists should have match for datatables compatibility >+ if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/match' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing 'match' parameter", $verb, $route ); >+ } >+ >+ # plural lists should have order_by for datatables compatibility >+ if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/order_by' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing 'order_by' parameter", $verb, $route ); >+ } >+ >+ # plural lists should have page for datatables compatibility >+ if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/page' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing 'page' parameter", $verb, $route ); >+ } >+ >+ # plural lists should have per_page for datatables compatibility >+ if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/per_page' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing 'per_page' parameter", $verb, $route ); >+ } >+ >+ # plural lists should have request_id_header for datatables compatibility >+ if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/request_id_header' } @parameters ) { >+ push @errors, sprintf( "%s %s -> missing request_id_header parameter", $verb, $route ); >+ } >+ } >+ } >+ } >+ >+ is( scalar @errors, 0, 'No errors in GET endpoints definitions in the spec' ); >+ >+ foreach my $error (@errors) { >+ print STDERR "$error\n"; >+ } >+}; >+ > subtest 'POST (201) have location header' => sub { > my @files = `git ls-files 'Koha/REST/V1/**/*.pm'`; > plan tests => scalar @files; >-- >2.49.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 39817
: 181869