|
Description
Pedro Amorim
2023-03-14 15:42:54 UTC
Created attachment 148213 [details] [review] Bug 32227: Unit tests This patch adds a unit test that launches the swagger-cli validator against our spec. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 148214 [details] [review] Bug 33227: Unit tests This patch adds a unit test that launches the swagger-cli validator against our spec. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> This is great! It'll be a vast improvement to have our spec validated regularly like this :) Confirmed working from this end so feel free to add my stamp if you're ready, Tomas. Otherwise, I'll wait for the bug to go NSO. Created attachment 148268 [details] [review] Bug 33227: Unit tests This patch adds a unit test that launches the swagger-cli validator against our spec. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 148269 [details] [review] Bug 33227: Remove invalid spec and adjust the code accordingly The spec contains a non-standard definition for the body param. Removing it from the spec makes us need to handle the 'body' manually in the case of JSON data. This patch basically does that. It also changes the uses of $c->validation, which are discouraged by the Mojolicious::Plugin::OpenAPI dev/maintainer. I do it to highlight what we must do in other places and the fact that there's no behavior change. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Created attachment 148407 [details] [review] Bug 33227: Unit tests This patch adds a unit test that launches the swagger-cli validator against our spec. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 148408 [details] [review] Bug 33227: Remove invalid spec and adjust the code accordingly The spec contains a non-standard definition for the body param. Removing it from the spec makes us need to handle the 'body' manually in the case of JSON data. This patch basically does that. It also changes the uses of $c->validation, which are discouraged by the Mojolicious::Plugin::OpenAPI dev/maintainer. I do it to highlight what we must do in other places and the fact that there's no behavior change. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> This resolves the issues presented and causes now regressions. However, I'd love to see some documentation added somewhere to the API docs that states we expect MARC records to be passed (in various formats).. at the moment, the docs aren't all that clear. Anyways.. Passing QA to get us back on spec Created attachment 148410 [details] [review] Bug 33227: (follow-up) Improve docs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 148411 [details] [review] Bug 33227: (follow-up) Improve docs Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x Doesn't apply cleanly in 22.05.x, if needed please rebase. xt/api.t is currently failing but the failure is hidden because we skip the test (swagger-cli is missing from the node_modules in ktd). Why not simply fail if missing? I think I agree.. lets remove the SKIP set the test as failed Created attachment 168707 [details] [review] Bug 33227: (follow-up) Set test to failed if swagger-cli missing I'm going to move that to it's own bug See bug 37302 |