Description
Lari Taskula
2016-08-02 16:33:07 UTC
Created attachment 53936 [details] [review] Bug 17032: Make sure Swagger object definition is up-to-date with Koha-object If a definition in api/v1/definitions/*.json has a corresponding Koha-object by filename, compare properties of the definition and columns of the Koha-object and ensure that all columns are defined inside properties of the definition. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. See that all of the tests pass. (If not, file a bug report to fix missing properties in the Swagger definition.) 3. Remove a property from api/v1/swagger/definitions/patron.json 4. Repeat step 1 5. See that test fails and lets you know that the property you just removed is missing from the definition. Comment on attachment 53936 [details] [review] Bug 17032: Make sure Swagger object definition is up-to-date with Koha-object Review of attachment 53936 [details] [review]: ----------------------------------------------------------------- ::: t/db_dependent/api/v1/swagger/definitions.t @@ +72,5 @@ > + > + $name = "Koha::" . ucfirst $name; > + > + return eval { > + Module::Load::load($name); The correct way to do that is to use Module::Load::Conditional. Created attachment 53963 [details] [review] Bug 17032: Make sure Swagger object definition is up-to-date with Koha-object (In reply to Jonathan Druart from comment #2) > The correct way to do that is to use Module::Load::Conditional. As requested, patch above. By the way, at the moment tests are failing, because patron.json is missing 2 properties and hold.json is missing one. Apply bugs Bug 17041 and Bug 17042 to run this cleanly (also sign-off for them will be appreciated :). Created attachment 53980 [details] [review] Bug 17032: Make sure Swagger object definition is up-to-date with Koha-object If a definition in api/v1/definitions/*.json has a corresponding Koha-object by filename, compare properties of the definition and columns of the Koha-object and ensure that all columns are defined inside properties of the definition. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. See that all of the tests pass. (If not, file a bug report to fix missing properties in the Swagger definition.) 3. Remove a property from api/v1/swagger/definitions/patron.json 4. Repeat step 1 5. See that test fails and lets you know that the property you just removed is missing from the definition. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Works as intended! Good work! Created attachment 54022 [details] [review] Bug 17032: Make sure Swagger object definition is up-to-date with Koha-object If a definition in api/v1/definitions/*.json has a corresponding Koha-object by filename, compare properties of the definition and columns of the Koha-object and ensure that all columns are defined inside properties of the definition. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. See that all of the tests pass. (If not, file a bug report to fix missing properties in the Swagger definition.) 3. Remove a property from api/v1/swagger/definitions/patron.json 4. Repeat step 1 5. See that test fails and lets you know that the property you just removed is missing from the definition. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 54023 [details] [review] Bug 17032: Remove warning "keys on reference is experimental" Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Thanks Lari! In case this test doesn't always return the same result, and fails sometimes, you might be interested in this: I applied many API-related patches on top of master and ran this test multiple times. I noticed it didn't always have the same result; Swagger2 wasn't always able to find the type of definition because for some reason $ref was left unresolved for that definition - but after updating to the latest versions of Swagger2 (0.83) and JSON::Validator (0.82), it now seems to work. Looks like the author of JSON::Validator just patched some issues regarding to resolving $refs. Created attachment 55437 [details] [review] Bug 17032 [QA Followup] - Update path to swagger.json file Pushed to master for 16.11, thanks Lari, Jonathan! Enhancement, skipping for 16.05.x |