Adding semantic and model validator to CI #910
Conversation
amarzavery
commented
Feb 5, 2017
•
amarzavery
commented
- Refactored common code to util/utils.js
- Added semantic mode with PR_ONLY false/true
- Added semantic mode to allowed failures
- Upgraded the version of node.js to 6.x
- modified package.json to accurately reflect the devDependencies
- added support for running CI in custom branch when a PR is sent to that branch
package.json
Outdated
| "devDependencies": { | ||
| "mocha": "*" | ||
| "z-schema": "^3.16.1", | ||
| "openapi-validation-tools": "amarzavery/openapi-validation-tools#sway4" |
There was a problem hiding this comment.
Curious: Why are we using amarzavery fork of openapi-validation-tools of sway?
There was a problem hiding this comment.
will update to azure/openapi-validation-tools#master pretty soon.
| @@ -31,4 +30,4 @@ | |||
| "scripts": { | |||
| "test": "mocha -t 500000" | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Minor: I am not sure what is the general rule for line ending in JavaScript world?
There was a problem hiding this comment.
There is no rule as such.
| } | ||
| var _ = require('lodash'), | ||
| execSync = require('child_process').execSync, | ||
| utils = require('./util/utils'); |
There was a problem hiding this comment.
Cool. Thanks for refactoring the code into utils 👍
test/semantic.js
Outdated
| utils = require('./util/utils'), | ||
| oav = require('openapi-validation-tools'); | ||
|
|
||
| describe('Azure swagger semantic validation:', function () { |
There was a problem hiding this comment.
Minor: (Nit-pick) two spaces between semantic & validation: instead of one.
test/semantic.js
Outdated
| let swaggersToProcess = utils.getFilesChangedInPR(); | ||
| _(swaggersToProcess).each(function (swagger) { | ||
| it(swagger + ' should be semantically valid.', function (done) { | ||
| oav.validateSpec(swagger, false, 'error').catch(function (err) { |
There was a problem hiding this comment.
curious: What is the second parameter in validateSpec function?
There was a problem hiding this comment.
The second parameter is whether you want a detailed json output or not. I have put this to false as I dont want the CI logs to be lengthy.
test/test.js
Outdated
| var error = context.validator.getLastErrors(); | ||
| throw new Error("Schema validation for Composite Swagger failed: " + util.inspect(error, { depth: null })); | ||
| } | ||
| assert(valid === true); |
There was a problem hiding this comment.
What are the possible values of valid? I am asking because i was thinking whether we can remove this line or not?
There was a problem hiding this comment.
valid will always be a boolean value. That is returned from validation using z-schema validator package. We would want to fail the CI if swagger spec is not a valid swagger spec. Hence the assert statement.
test/test.js
Outdated
| done(); | ||
| describe('Azure x-ms-example schema validation:', function () { | ||
| _(utils.examples).each(function (example) { | ||
| it('x-ms-examples: ' + example + ' should be a valid x-ms-example.', function (done) { |
There was a problem hiding this comment.
This is schema validation for x-ms-examples. This will run only if x-ms-examples is present.
| exampleSchema: exampleSchemaBody, | ||
| compositeSchema: compositeSchemaBody | ||
| }; | ||
| let validator = new z({ breakOnFirstError: false }); |
There was a problem hiding this comment.
Curious: Newbie here :) What is new z?
There was a problem hiding this comment.
z is the z-schema package that we depend on for validating a json document against a given schema.
Top of the file should have a variable named z defined.
amarzavery
changed the title
|
@vishrutshah - Please review and merge :). |
|
No modification for Ruby |
|
No modification for Python |
|
No modification for NodeJS |
