Bug 35218

Summary: No tests for /erm/sushi_service
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: CLOSED FIXED QA Contact:
Severity: blocker    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, tomascohen
Version: unspecifiedKeywords: rel_23_11_candidate
Hardware: All   
OS: All   
GIT URL: 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:
Bug Depends on: 34587    
Bug Blocks:    
Attachments: Bug 35218: Add unit tests for the counter registry API
Bug 35218: Add unit tests for the counter registry API
Bug 35218: Add unit tests for the counter registry API
Bug 35218: (QA follow-up) Perltidy and file permissions

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