A new field has been added to the response object for the sushi service API, this needs updating in our specs
Created attachment 177381 [details] [review] Bug 39007: Update API spec and unit test
Created attachment 177382 [details] [review] Bug 39007: Update API spec and unit test Test plan: 1) Run prove t/db_dependent/api/v1/erm_sushi_services.t - FAIL 2) Apply patch 3) Repeat step 1 - PASS 4) Inspect patch diff and note that last_audit has been added to the API definition
Created attachment 177390 [details] [review] Bug 39007: Update API spec and unit test Test plan: 1) Run prove t/db_dependent/api/v1/erm_sushi_services.t - FAIL 2) Apply patch 3) Repeat step 1 - PASS 4) Inspect patch diff and note that last_audit has been added to the API definition Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Fixed inline: FAIL t/db_dependent/api/v1/erm_sushi_services.t FAIL spelling repsonse ==> response Could you confirm if the QA script is showing this fail for you or not? I have been suspecting it might depend on some setup.
Thanks for the quick work! Picking this up for main directly.
Created attachment 177392 [details] [review] Bug 39007: (follow-up) Fix typo
(In reply to Katrin Fischer from comment #4) > Fixed inline: > > FAIL t/db_dependent/api/v1/erm_sushi_services.t > FAIL spelling > repsonse ==> response > > Could you confirm if the QA script is showing this fail for you or not? I > have been suspecting it might depend on some setup. Its a typo, added a follow-up, feel free to squash it if you haven't already pushed the first patch to main
(In reply to Matt Blenkinsop from comment #7) > (In reply to Katrin Fischer from comment #4) > > Fixed inline: > > > > FAIL t/db_dependent/api/v1/erm_sushi_services.t > > FAIL spelling > > repsonse ==> response > > > > Could you confirm if the QA script is showing this fail for you or not? I > > have been suspecting it might depend on some setup. > > Its a typo, added a follow-up, feel free to squash it if you haven't already > pushed the first patch to main Already pushed, or more fixed. Hope that's ok. I was just really interested since I get a lof of those misses if the QA script always detects it.
Pushed for 25.05! Well done everyone, thank you!
The test will always pass, it's using warning_like, it does not make sense.
Created attachment 177463 [details] [review] Bug 39007: Replace warning_like with is
I am also wondering why we are comparing using a "fake" array. If last_audit is removed from the api spec, the test is still passing. You only need to add it to the @expected_fields list in the .t file. It would be more robust to actually compare with the response of our route, or (if too complicated) with the spec directly.
Is your follow-up ready to be pushed or better wait?
(In reply to Katrin Fischer from comment #13) > Is your follow-up ready to be pushed or better wait? The test is useless without the follow-up as it will always pass.
Picked "Repalye warning_like with is" for next push to main.
Hi all, We are getting a sushi error on jenkins ( https://jenkins.koha-community.org/job/Koha_Sec/job/Koha_24.05/lastCompletedBuild/testReport/(root)/t_db_dependent_api_v1_erm_sushi_services_t/get___tests__2270_/ ) and we have been told to apply this patchfix. When we apply it is not applying cleanly, see below: kohadev-koha@kohadevbox:koha((a5afd9c9c8b...)|AM 1/1)$ sed -n '/<<<<</,/>>>>>/p' t/db_dependent/api/v1/erm_sushi_services.t <<<<<<< HEAD is( scalar(@new_fields_in_response), 0, 'The response fields match the expected fields' ); ======= my $new_fields_string = "This is not a new error within Koha, the following new field(s) have been added to the API repsonse: " . join( ', ', @new_fields_in_response ) . '. They should be added to the API definition'; warning_like { scalar(@new_fields_in_response) } 0, $new_fields_string; $schema->storage->txn_rollback; >>>>>>> Bug 39007: Update API spec and unit test The difference is the $schema->storage->txn_rollback; line which is introduced by bug 37898 Could someone please rebase for 24.05 or advise what I should do? Thanks, Alex
Created attachment 178004 [details] [review] Bug 39007: [24.05] Update API spec and unit test Test plan: 1) Run prove t/db_dependent/api/v1/erm_sushi_services.t - FAIL 2) Apply patch 3) Repeat step 1 - PASS 4) Inspect patch diff and note that last_audit has been added to the API definition Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Please enter the commit message for your changes. Lines starting
Created attachment 178005 [details] [review] Bug 39007: [24.05] (follow-up) Fix typo
Created attachment 178006 [details] [review] Bug 39007: [24.05] Replace warning_like with is
Thank you for the rebased patches Jonathan
Applied to 24.05.x-security
Applied to 23.11.x-security
Hi RMaints, can you please update the "Versions released in"?
(In reply to Katrin Fischer from comment #23) > Hi RMaints, can you please update the "Versions released in"? All done, thanks Katrin :)