Attachment 121286 Details for Bug 28370 – Bug 28370: Rewrite tests

[patch] Bug 28370: Rewrite tests

Bug-28370-Rewrite-tests.patch (text/plain), 5.30 KB, created by Tomás Cohen Arazi (tcohen) on 2021-05-21 20:21:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2021-05-21 20:21:47 UTC

Size: 5.30 KB

patch

obsolete

>From b30470428cea08dfe2a7eec9cbbda30ab8029da5 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 21 May 2021 17:05:27 -0300
>Subject: [PATCH] Bug 28370: Rewrite tests
>
>This patch rewrites the tests, and also doesn't enforce
>additionalProperties to be false. We could need it, and there are routes
>that need to be reviewed that would break otherwise now: error.json, for
>example, is used everywhere, and some routes add some payload to it.
>
>The main change this patch introduces is using the resolved spec instead
>of finding each .json file, which wasn't even correct as we have .yaml
>files already. Parameters and responses are tested.
>
>To test:
>1. Apply this patch
>2. Run:
>   $ kshell
>  k$ prove xt/api.t
>=> SUCCESS: Tests fail! A nice report on the failures is printed
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> xt/api.t | 95 ++++++++++++++++++++++++++++----------------------------
> 1 file changed, 48 insertions(+), 47 deletions(-)
>
>diff --git a/xt/api.t b/xt/api.t
>index 0a64bb422af..3d33a669bff 100755
>--- a/xt/api.t
>+++ b/xt/api.t
>@@ -13,63 +13,64 @@
> # with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>-use File::Slurp qw( read_file );
>-use JSON qw( from_json );
>-use Test::More tests => 4;
>+
>+use Test::More tests => 2;
>+
>+use Test::Mojo;
> use Data::Dumper;
> 
>-my $dh;
>+my $t    = Test::Mojo->new('Koha::REST::V1');
>+my $spec = $t->get_ok( '/api/v1/', 'Correctly fetched the spec' )->tx->res->json;
> 
>-my $definitions_dir = 'api/v1/swagger/definitions';
>-opendir( $dh, $definitions_dir ) or die "$!";
>-my @files = readdir $dh;
>-my @wrong_additionalProperties;
>-ok( @files, "making sure we found definitions files" );
>-for my $file (@files) {
>-    next unless $file =~ m|\.json$|;
>-    my $spec = from_json read_file("$definitions_dir/$file");
>-    if ( not exists $spec->{additionalProperties}
>-        or $spec->{additionalProperties} != 0 )
>-    {
>-        push @wrong_additionalProperties, { file => $file, };
>-    }
>-}
>-is( scalar @wrong_additionalProperties, 0 )
>-  or diag Dumper \@wrong_additionalProperties;
>+my $paths = $spec->{paths};
>+
>+my @missing_additionalProperties = ();
> 
>+foreach my $route ( keys %{$paths} ) {
>+    foreach my $verb ( keys %{ $paths->{$route} } ) {
> 
>-my $paths_dir = 'api/v1/swagger/paths';
>-opendir( $dh, $paths_dir ) or die "$!";
>-@files = readdir $dh;
>-@wrong_additionalProperties = ();
>-ok(@files, "making sure we found paths files");
>-for my $file (@files) {
>-    next unless $file =~ m|\.json$|;
>-    my $spec = from_json read_file("$paths_dir/$file");
>-    for my $route ( keys %$spec ) {
>-        for my $method ( keys %{ $spec->{$route} } ) {
>-            next if $method ne 'post' && $method ne 'put';
>-            for my $parameter ( @{ $spec->{$route}->{$method}->{parameters} } ) {
>-                if ( exists $parameter->{schema} ) {
>+        # p($paths->{$route}->{$verb});
>+
>+        # check parameters []
>+        foreach my $parameter ( @{ $paths->{$route}->{$verb}->{parameters} } ) {
>+            if (   exists $parameter->{schema}
>+                && exists $parameter->{schema}->{type}
>+                && ref( $parameter->{schema}->{type} ) ne 'ARRAY'
>+                && $parameter->{schema}->{type} eq 'object' ) {
>+
>+                # it is an object type definition
>+                if ( not exists $parameter->{schema}->{additionalProperties} ) {
>+                    push @missing_additionalProperties,
>+                      { type  => 'parameter',
>+                        route => $route,
>+                        verb  => $verb,
>+                        name  => $parameter->{name}
>+                      };
>+                }
>+            }
>+        }
> 
>-                    # If it's a ref we inherit from the definition file
>-                    next if exists $parameter->{schema}->{'$ref'};
>+        # check responses  {}
>+        my $responses = $paths->{$route}->{$verb}->{responses};
>+        foreach my $response ( keys %{$responses} ) {
>+            if (   exists $responses->{$response}->{schema}
>+                && exists $responses->{$response}->{schema}->{type}
>+                && ref( $responses->{$response}->{schema}->{type} ) ne 'ARRAY'
>+                && $responses->{$response}->{schema}->{type} eq 'object' ) {
> 
>-                    if ( not exists $parameter->{schema}->{additionalProperties}
>-                        or $parameter->{schema}->{additionalProperties} != 0 )
>-                    {
>-                        push @wrong_additionalProperties,
>-                          {
>-                            file   => "$paths_dir/$file",
>-                            route  => $route,
>-                            method => $method,
>-                          };
>-                    }
>+                # it is an object type definition
>+                if ( not exists $responses->{$response}->{schema}->{additionalProperties} ) {
>+                    push @missing_additionalProperties,
>+                      { type  => 'response',
>+                        route => $route,
>+                        verb  => $verb,
>+                        name  => $response
>+                      };
>                 }
>             }
>         }
>     }
> }
> 
>-is( scalar @wrong_additionalProperties, 0 )
>-  or diag Dumper \@wrong_additionalProperties;
>+is( scalar @missing_additionalProperties, 0 )
>+  or diag Dumper \@missing_additionalProperties;
>-- 
>2.31.1

Actions: View | Diff | Splinter Review

Attachments on bug 28370: 121110 | 121134 | 121232 | 121270 | 121284 | 121285 | 121286 | 121287 | 121289