Bug 35218 - No tests for /erm/sushi_service
Summary: No tests for /erm/sushi_service
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low blocker
Assignee: Martin Renvoize (ashimema)
QA Contact:
URL:
Keywords: rel_23_11_candidate
Depends on: 34587
Blocks:
  Show dependency treegraph
 
Reported: 2023-11-01 17:59 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-01-04 09:07 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00
Circulation function:


Attachments
Bug 35218: Add unit tests for the counter registry API (12.34 KB, patch)
2023-11-06 14:17 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35218: Add unit tests for the counter registry API (12.37 KB, patch)
2023-11-06 19:27 UTC, David Nind
Details | Diff | Splinter Review
Bug 35218: Add unit tests for the counter registry API (12.42 KB, patch)
2023-11-06 19:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 35218: (QA follow-up) Perltidy and file permissions (11.60 KB, patch)
2023-11-06 19:34 UTC, Katrin Fischer
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 (tcohen) 2023-11-01 17:59:07 UTC
This response definition is weird:

      200:
        description: A sushi service
        schema:
          items:
            $ref: "../swagger.yaml#/definitions/erm_sushi_service"
          type: object

`items` applies to `array` types. When digging in the tests and code to find out wether it returns an array of objects, or an object, I found out there are actually no tests for this endpoint!

I'm removing the stray 'type: object' entry there because it makes some other tests fail. But please provide tests, and proof of the expected behavior.
Comment 1 Matt Blenkinsop 2023-11-06 14:17:31 UTC
Created attachment 158527 [details] [review]
Bug 35218: Add unit tests for the counter registry API

This patch adds two unit tests to cover the integration with the Counter Registry API in the Usage Statistics module. This is to prevent regressions caused either by changes to the Koha endpoint, or by changes to the Counter Registry API that might require changes in Koha to handle.

Test plan:
1) prove t/db_dependent/api/v1/erm_counter_registries.t
2) prove t/db_dependent/api/v1/erm_sushi_services.t
Comment 2 David Nind 2023-11-06 19:27:28 UTC
Created attachment 158545 [details] [review]
Bug 35218: Add unit tests for the counter registry API

This patch adds two unit tests to cover the integration with the Counter Registry API in the Usage Statistics module. This is to prevent regressions caused either by changes to the Koha endpoint, or by changes to the Counter Registry API that might require changes in Koha to handle.

Test plan:
1) prove t/db_dependent/api/v1/erm_counter_registries.t
2) prove t/db_dependent/api/v1/erm_sushi_services.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Katrin Fischer 2023-11-06 19:29:48 UTC
Love the quick work... but QA test tools!

 FAIL	t/db_dependent/api/v1/erm_counter_registries.t
   FAIL	  file permissions
		File must have the exec flag
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 42)

 FAIL	t/db_dependent/api/v1/erm_sushi_services.t
   FAIL	  file permissions
		File must have the exec flag
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 33)
Comment 4 Katrin Fischer 2023-11-06 19:34:34 UTC
Created attachment 158546 [details] [review]
Bug 35218: Add unit tests for the counter registry API

This patch adds two unit tests to cover the integration with the Counter Registry API in the Usage Statistics module. This is to prevent regressions caused either by changes to the Koha endpoint, or by changes to the Counter Registry API that might require changes in Koha to handle.

Test plan:
1) prove t/db_dependent/api/v1/erm_counter_registries.t
2) prove t/db_dependent/api/v1/erm_sushi_services.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2023-11-06 19:34:38 UTC
Created attachment 158547 [details] [review]
Bug 35218: (QA follow-up) Perltidy and file permissions

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2023-11-06 19:35:01 UTC
I perltidied the whole test files, but still got stuck with a few missing lines?
Comment 7 Tomás Cohen Arazi (tcohen) 2023-11-08 13:00:08 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 8 Fridolin Somers 2023-11-10 07:58:01 UTC
Depends on Bug 34587 not in 23.05.x
Comment 9 Jonathan Druart 2024-01-04 09:07:16 UTC
10:01 < Joubu> t/db_dependent/api/v1/erm_sushi_services.t is failing because it compares an expected output with https://registry.projectcounter.org/api/v1/sushi-service/b94bc981-fa16-4bf6-ba5f-6c113f7ffa0b/
10:01 < Joubu> and there is "migrations" there
10:01 < Joubu> which may have been added recently
10:01 < Joubu> this test is really bad (ie. depending on an external resource)
10:02 < Joubu> unless it's what we want, but it's not clear what the test is actually testing