[FEATURE] Add Configuration Schema #274
Conversation
|
Should be rebased on top of #277 |
| const escapeStringRegExp = require("escape-string-regexp"); | ||
| const matchAll = require("string.prototype.matchall"); | ||
|
|
||
| class ValidationError extends Error { |
There was a problem hiding this comment.
I am really missing some jsdoc here for the class and methods.
| "properties": { | ||
| "specVersion": { "enum": ["2.0"] }, | ||
| "kind": { | ||
| "enum": ["project", null] |
There was a problem hiding this comment.
What is the enum entry "null" for?
There was a problem hiding this comment.
The default when it's not defined. I've added comments.
| return JSON.parse(schemaFile); | ||
| } | ||
|
|
||
| class Validator { |
There was a problem hiding this comment.
Same here, no documentation. Most of the methods logic is obvious but at least the class could use some doc.
test/lib/projectPreprocessor.js
Outdated
| }); | ||
|
|
||
| t.is(validationError.errors.length, 1, "ValidationError should have one error object"); | ||
| t.is(validationError.errors.length, 1, "ValidationError should have one error object"); |
lib/projectPreprocessor.js
Outdated
| @@ -3,8 +3,9 @@ const fs = require("graceful-fs"); | |||
| const path = require("path"); | |||
| const {promisify} = require("util"); | |||
| const readFile = promisify(fs.readFile); | |||
| const parseYaml = require("js-yaml").safeLoadAll; | |||
| const {loadAll: parseYaml, DEFAULT_SAFE_SCHEMA} = require("js-yaml"); | |||
There was a problem hiding this comment.
This change is needed because of the issues with safeLoadAll? If yes, maybe add a comment for that.
There was a problem hiding this comment.
projectPreprocessor » readConfigFile: Exception for invalid config failed on Windows: https://dev.azure.com/sap/opensource/_build/results?buildId=47033&view=ms.vss-test-web.build-test-results-tab&runId=1017232&resultId=100193&paneView=debug
lib/validation/validator.js
Outdated
| * @param {number} [options.yaml.documentIndex=0] Document index in case the YAML file contains multiple documents | ||
| * @throws {module:@ui5/project.validation.ValidationError} | ||
| * Throws a {@link module:@ui5/project.validation.ValidationError ValidationError} when the validation fails. | ||
| * @returns {undefined} Returns when the validation succeeds |
|
Tests should be fixed now. |
| const yaml = { | ||
| path: "/my-project/ui5.yaml", | ||
| source: | ||
| `--- |
There was a problem hiding this comment.
What about one (or multiple) newlines at the beginning of the file?
| source: configFile, | ||
| documentIndex | ||
| configs.map(async (config, documentIndex) => { | ||
| try { |
There was a problem hiding this comment.
We should document that the try/catch block here is required to get the first error of the first document. And not the first error to occur in any document.
| schemaPath: "#/type", | ||
| }; | ||
|
|
||
| const expectedErrorMessage = `Configuration ${chalk.underline(chalk.red("foo"))} should be of type 'object'`; |
| schemaPath: "#/required", | ||
| }; | ||
|
|
||
| const expectedErrorMessage = "Configuration should have required property 'specVersion'"; |
There was a problem hiding this comment.
"Configuration must have the required property 'specVersion'"
| message: "should have required property 'name'" | ||
| }; | ||
|
|
||
| const expectedErrorMessage = `Configuration ${chalk.underline(chalk.red("metadata"))} should have required property 'name'`; |
There was a problem hiding this comment.
"Configuration ${chalk.underline(chalk.red("metadata"))} must have the required property 'name'"
| }; | ||
|
|
||
| const expectedErrorMessage = | ||
| `Configuration ${chalk.underline(chalk.red("type"))} should be equal to one of the allowed values |
There was a problem hiding this comment.
"Configuration ${chalk.underline(chalk.red("type"))} must be equal to one of the allowed values"
| }; | ||
|
|
||
| const expectedErrorMessage = | ||
| `Configuration ${chalk.underline(chalk.red("resources/configuration"))} property propertiesFileEncoding is not expected to be here`; |
There was a problem hiding this comment.
"Configuration ${chalk.underline(chalk.red("resources/configuration"))} property propertiesFileEncoding must not be provided here"
Thank you for your contribution! 🙌
To get it merged faster, kindly review the checklist below:
Pull Request Checklist