From 6d239ed258683fae170a1afc6a88ff4c0db8ef3a Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:45:05 -0700 Subject: [PATCH 01/15] chore: update validate_packs for testing * exported method for testing and added a guard to prevent file from executing when used in test. --- utils/validate_packs.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/utils/validate_packs.js b/utils/validate_packs.js index e9fbbbda79..391af21253 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -111,5 +111,9 @@ const main = () => { } } -main(); +if (require.main === module) { + main(); +} + +module.exports = { validateFile }; From fc1a733567818155e1cd63aeb11d2fd5884c823f Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:45:07 -0700 Subject: [PATCH 02/15] chore: adding tests for validate_packs --- utils/__tests__/validate_packs.test.js | 296 +++++++++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 utils/__tests__/validate_packs.test.js diff --git a/utils/__tests__/validate_packs.test.js b/utils/__tests__/validate_packs.test.js new file mode 100644 index 0000000000..8213082248 --- /dev/null +++ b/utils/__tests__/validate_packs.test.js @@ -0,0 +1,296 @@ +const { validateFile } = require('../validate_packs'); + +jest.spyOn(global.console, 'log').mockImplementation(() => {}); +jest.spyOn(global.console, 'error').mockImplementation(() => {}); + +const getTestFile = (schemaType) => { + const files = { + alert: { + path: '/alerts/', + contents: [ + { + name: 'fakealert', + type: 'STATIC', + nrql: { + query: + "FROM ElasticsearchClusterSample SELECT uniqueCount(displayName) WHERE cluster.status = 'red' FACET displayName", + }, + }, + ], + }, + dashboard: { + path: '/dashboards/', + contents: [ + { + name: 'fakedashboard', + }, + ], + }, + flex: { + path: '/instrumentation/flex/', + contents: [ + { + name: 'fakeflexconfig', + }, + { + integrations: [], + }, + ], + }, + synthetic: { + path: '/instrumentation/synthetics', + contents: [ + { + name: 'fakesynthetic', + }, + ], + }, + main_config: { + path: '/main_config/', // this can be any path + contents: [ + { + name: 'fakeobservabilitypack', + }, + ], + }, + }; + + return files[schemaType]; +}; + +describe('test schema validation', () => { + describe('alert validation', () => { + test.each` + type + ${'STATIC'} + ${'BASELINE'} + ${'OUTLIER'} + `('doesnt fail for a valid alert definition', ({ type }) => { + const alertTestFile = getTestFile('alert'); + alertTestFile.contents[0].type = type; + + const { errors } = validateFile(alertTestFile); + + expect(errors).toEqual([]); + }); + + test('detects empty alert definition', () => { + const alertTestFile = getTestFile('alert'); + alertTestFile.contents = [{}]; + + const { errors } = validateFile(alertTestFile); + + expect(errors.length).toBeGreaterThan(0); + }); + + test('detects alert definition that is missing name', () => { + const alertTestFile = getTestFile('alert'); + delete alertTestFile.contents[0].name; + + const { errors } = validateFile(alertTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects alert definition that is missing type', () => { + const alertTestFile = getTestFile('alert'); + delete alertTestFile.contents[0].type; + + const { errors } = validateFile(alertTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects alert definition with non-allowed type', () => { + const alertTestFile = getTestFile('alert'); + alertTestFile.contents[0].type = 'INVALID_TYPE'; + + const { errors } = validateFile(alertTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects alert definition that is missing nrql', () => { + const alertTestFile = getTestFile('alert'); + delete alertTestFile.contents[0].nrql; + + const { errors } = validateFile(alertTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects alert definition that is missing nrql.query', () => { + const alertTestFile = getTestFile('alert'); + delete alertTestFile.contents[0].nrql.query; + + const { errors } = validateFile(alertTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects alert definition that is missing multiple fields', () => { + const alertTestFile = getTestFile('alert'); + delete alertTestFile.contents[0].name; + delete alertTestFile.contents[0].type; + delete alertTestFile.contents[0].nrql; + + const { errors } = validateFile(alertTestFile); + + console.dir(errors); + expect(errors.length).toBe(3); + }); + }); + + describe('dashboard validation', () => { + test('doesnt fail for valid dashboard definition', () => { + const dashboardTestFile = getTestFile('dashboard'); + + const { errors } = validateFile(dashboardTestFile); + + expect(errors).toEqual([]); + }); + + test('detects empty dashboard definition', () => { + const dashboardTestFile = getTestFile('dashboard'); + dashboardTestFile.contents = [{}]; + + const { errors } = validateFile(dashboardTestFile); + + expect(errors.length).toBeGreaterThan(0); + }); + + test('detects dashboard definition that is missing name', () => { + const dashboardTestFile = getTestFile('dashboard'); + delete dashboardTestFile.contents[0].name; + + const { errors } = validateFile(dashboardTestFile); + + expect(errors.length).toBe(1); + }); + }); + + describe('flex validation', () => { + test('doesnt fail for valid flex definition', () => { + const flexTestFile = getTestFile('flex'); + + const { errors } = validateFile(flexTestFile); + + expect(errors).toEqual([]); + }); + + test('detects empty flex config', () => { + const flexTestFile = getTestFile('flex'); + flexTestFile.contents[0] = {}; // contents[0] is the config + + const { errors } = validateFile(flexTestFile); + + expect(errors.length).toBeGreaterThan(0); + }); + + test('detects empty flex integration', () => { + const flexTestFile = getTestFile('flex'); + flexTestFile.contents[1] = {}; // contents[1] is the integration + + const { errors } = validateFile(flexTestFile); + + expect(errors.length).toBeGreaterThan(0); + }); + + test('detects flex config definition that is missing name', () => { + const flexTestFile = getTestFile('flex'); + delete flexTestFile.contents[0].name; + + const { errors } = validateFile(flexTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects flex integration definition that is missing integrations', () => { + const flexTestFile = getTestFile('flex'); + delete flexTestFile.contents[1].integrations; + + const { errors } = validateFile(flexTestFile); + + expect(errors.length).toBe(1); + }); + }); + + describe('synthetic validation', () => { + test('doesnt fail for valid synthetic definition', () => { + const syntheticTestFile = getTestFile('synthetic'); + + const { errors } = validateFile(syntheticTestFile); + + expect(errors).toEqual([]); + }); + + test('detects empty synthetic definition', () => { + const syntheticTestFile = getTestFile('synthetic'); + syntheticTestFile.contents = [{}]; + + const { errors } = validateFile(syntheticTestFile); + + expect(errors.length).toBeGreaterThan(0); + }); + + test('detects synthetic definition that is missing name', () => { + const syntheticTestFile = getTestFile('synthetic'); + delete syntheticTestFile.contents[0].name; + + const { errors } = validateFile(syntheticTestFile); + + expect(errors.length).toBe(1); + }); + }); + + describe('main config validation', () => { + test.each` + level + ${'New Relic'} + ${'Verified'} + ${'Community'} + `('doesnt fail for a valid alert definition', ({ level }) => { + const mainConfigTestFile = getTestFile('main_config'); + mainConfigTestFile.contents[0].level = level; + + const { errors } = validateFile(mainConfigTestFile); + + expect(errors).toEqual([]); + }); + + test('doesnt fail for valid alert definition w/o non-required fields', () => { + const mainConfigTestFile = getTestFile('main_config'); + + const { errors } = validateFile(mainConfigTestFile); + + expect(errors).toEqual([]); + }); + + test('detects empty main config definition', () => { + const mainConfigTestFile = getTestFile('main_config'); + mainConfigTestFile.contents = [{}]; + + const { errors } = validateFile(mainConfigTestFile); + + expect(errors.length).toBeGreaterThan(0); + }); + + test('detects main config definition that is missing name', () => { + const mainConfigTestFile = getTestFile('main_config'); + delete mainConfigTestFile.contents[0].name; + + const { errors } = validateFile(mainConfigTestFile); + + expect(errors.length).toBe(1); + }); + + test('detects invalid values for level field', () => { + const mainConfigTestFile = getTestFile('main_config'); + mainConfigTestFile.contents[0].level = 'INVALID_TYPE'; + + const { errors } = validateFile(mainConfigTestFile); + + expect(errors.length).toBe(1); + }); + }); +}); From 9749f70027d7531edf946c362a4a363e580a1f49 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:45:07 -0700 Subject: [PATCH 03/15] chore: fix bug * previously, validation was only returning one error -- even if there were multiple. instead, now returning all found errors. --- utils/validate_packs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/validate_packs.js b/utils/validate_packs.js index 391af21253..e45704d581 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -2,7 +2,7 @@ const path = require('path'); const glob = require('glob'); const Ajv = require('ajv'); -const ajv = new Ajv(); +const ajv = new Ajv({ allErrors: true }); const { readPackFile, removeCWDPrefix } = require('./helpers'); From 50a3a6016d654c60e7165fac06c540fcd31a90d6 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:45:08 -0700 Subject: [PATCH 04/15] chore: add method for converting errors, add test --- utils/__tests__/validate_packs.test.js | 75 ++++++++++++++++++++++++-- utils/validate_packs.js | 25 ++++++++- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/utils/__tests__/validate_packs.test.js b/utils/__tests__/validate_packs.test.js index 8213082248..29c97a556f 100644 --- a/utils/__tests__/validate_packs.test.js +++ b/utils/__tests__/validate_packs.test.js @@ -1,4 +1,5 @@ -const { validateFile } = require('../validate_packs'); +const { expect } = require('@jest/globals'); +const { validateFile, convertErrors } = require('../validate_packs'); jest.spyOn(global.console, 'log').mockImplementation(() => {}); jest.spyOn(global.console, 'error').mockImplementation(() => {}); @@ -58,7 +59,7 @@ const getTestFile = (schemaType) => { return files[schemaType]; }; -describe('test schema validation', () => { +describe('test validateFile', () => { describe('alert validation', () => { test.each` type @@ -136,7 +137,6 @@ describe('test schema validation', () => { const { errors } = validateFile(alertTestFile); - console.dir(errors); expect(errors.length).toBe(3); }); }); @@ -294,3 +294,72 @@ describe('test schema validation', () => { }); }); }); + +describe('test converErrors', () => { + test('converts enum error message', () => { + const mockAjvErrors = [ + { + instancePath: '/type', + schemaPath: '#/properties/type/enum', + keyword: 'enum', + params: { allowedValues: ['STATIC', 'BASELINE', 'OUTLIER'] }, + message: 'must be equal to one of the allowed values', + }, + ]; + + const convertedErrors = convertErrors(mockAjvErrors); + + expect(convertedErrors[0].message).toBe( + '[/type] must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]' + ); + }); + + test('doesnt convert message for non-enum error types', () => { + const mockAjvErrors = [ + { + instancePath: '', + schemaPath: '#/required', + keyword: 'required', + params: { missingProperty: 'name' }, + message: "must have required property 'name'", + }, + ]; + + const convertedErrors = convertErrors(mockAjvErrors); + + expect(convertedErrors[0].message).toBe( + "must have required property 'name'" + ); + }); + + test('removes fields that are not message', () => { + const mockAjvErrors = [ + { + instancePath: '/type', + schemaPath: '#/properties/type/enum', + keyword: 'enum', + params: { allowedValues: ['STATIC', 'BASELINE', 'OUTLIER'] }, + message: 'must be equal to one of the allowed values', + }, + { + instancePath: '', + schemaPath: '#/required', + keyword: 'required', + params: { missingProperty: 'name' }, + message: "must have required property 'name'", + }, + ]; + + const convertedErrors = convertErrors(mockAjvErrors); + + expect(convertedErrors).toEqual([ + { + message: + '[/type] must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]', + }, + { + message: "must have required property 'name'", + }, + ]); + }); +}); diff --git a/utils/validate_packs.js b/utils/validate_packs.js index e45704d581..b2c48f4c94 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -2,6 +2,7 @@ const path = require('path'); const glob = require('glob'); const Ajv = require('ajv'); +const { ErrorObject } = require('ajv'); const ajv = new Ajv({ allErrors: true }); const { readPackFile, removeCWDPrefix } = require('./helpers'); @@ -20,6 +21,26 @@ const EXCLUDED_DIRECTORY_PATTERNS = [ '*', ]; +/** + * Converts errors generated from ajv into 'general errors' we want to display to the user. + * @param {ErrorObject[]} ajvErrors - Errors generated from ajv validation. + * @return {Object[]} Array of our own internal error objects. + */ +const convertErrors = (ajvErrors) => { + const errors = ajvErrors.map((e) => { + if (e.keyword === 'enum') { + const message = `[${e.instancePath}] ${e.message}: ${JSON.stringify( + e.params.allowedValues + )}`; + return { message }; + } else { + return { message: e.message }; + } + }); + + return errors; +}; + /** * Validates an object against a JSON schema * @param {Object} content - The object to validate @@ -31,7 +52,7 @@ const validateAgainstSchema = (content, schema) => { const valid = validate(content); if (!valid) { - return validate.errors; + return convertErrors(validate.errors); } return []; @@ -115,5 +136,5 @@ if (require.main === module) { main(); } -module.exports = { validateFile }; +module.exports = { validateFile, convertErrors }; From d64fa12d6a3feaee89c85782d57097c2480efc5f Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:45:09 -0700 Subject: [PATCH 05/15] chore: update tests, move printing into its own method for testing * moved producing the output displayed to the user into its own method for future testing. * updated method which converts errors to also handle morphing the message for nested required fields. * updated tests. --- utils/__tests__/validate_packs.test.js | 44 ++++++++++++++++++------- utils/validate_packs.js | 45 +++++++++++++++++--------- 2 files changed, 63 insertions(+), 26 deletions(-) diff --git a/utils/__tests__/validate_packs.test.js b/utils/__tests__/validate_packs.test.js index 29c97a556f..4063b5bcf2 100644 --- a/utils/__tests__/validate_packs.test.js +++ b/utils/__tests__/validate_packs.test.js @@ -1,4 +1,3 @@ -const { expect } = require('@jest/globals'); const { validateFile, convertErrors } = require('../validate_packs'); jest.spyOn(global.console, 'log').mockImplementation(() => {}); @@ -307,14 +306,40 @@ describe('test converErrors', () => { }, ]; - const convertedErrors = convertErrors(mockAjvErrors); + const [convertedError, ..._] = convertErrors(mockAjvErrors); - expect(convertedErrors[0].message).toBe( - '[/type] must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]' + expect(convertedError.message).toBe( + `'/type' must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]` ); }); - test('doesnt convert message for non-enum error types', () => { + test('converts missing required field error message on a nested property', () => { + /* + previously if a field 'nrql' has a required field 'query', the message was: "must have required property 'query'". + there was no information about what level 'query' lived at (i.e, where does it go). + now it should look something like: "'/nrql' must have required property 'query'". + this should also be the case for fields nested beyond one level deep. + */ + + const mockAjvErrors = [ + { + instancePath: '/nrql', + schemaPath: '#/required', + keyword: 'required', + params: { missingProperty: 'query' }, + message: "must have required property 'query'", + }, + ]; + + const [convertedError, ..._] = convertErrors(mockAjvErrors); + + expect(convertedError.message).toBe( + `'/nrql' must have required property 'query'` + ); + }); + + // 'default' is anything except enum, and a missing required field on a nested field. + test('returns exact message for default case', () => { const mockAjvErrors = [ { instancePath: '', @@ -325,11 +350,9 @@ describe('test converErrors', () => { }, ]; - const convertedErrors = convertErrors(mockAjvErrors); + const [convertedError, ..._] = convertErrors(mockAjvErrors); - expect(convertedErrors[0].message).toBe( - "must have required property 'name'" - ); + expect(convertedError.message).toBe("must have required property 'name'"); }); test('removes fields that are not message', () => { @@ -354,8 +377,7 @@ describe('test converErrors', () => { expect(convertedErrors).toEqual([ { - message: - '[/type] must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]', + message: `'/type' must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]`, }, { message: "must have required property 'name'", diff --git a/utils/validate_packs.js b/utils/validate_packs.js index b2c48f4c94..8e9aa691a4 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -28,13 +28,20 @@ const EXCLUDED_DIRECTORY_PATTERNS = [ */ const convertErrors = (ajvErrors) => { const errors = ajvErrors.map((e) => { - if (e.keyword === 'enum') { - const message = `[${e.instancePath}] ${e.message}: ${JSON.stringify( - e.params.allowedValues - )}`; - return { message }; - } else { - return { message: e.message }; + console.log(e); + + let message = ''; + switch(true) { + case (e.keyword === 'enum'): + message = `'${e.instancePath}' ${e.message}: ${JSON.stringify( + e.params.allowedValues + )}`; + return { message }; + case (e.keyword === 'required' && e.instancePath != ''): + message = `'${e.instancePath}' ${e.message}`; + return { message }; + default: + return { message: e.message }; } }); @@ -113,19 +120,28 @@ const getPackFilePaths = (basePath) => { return [ ...yamlFilePaths, ...jsonFilePaths ]; } +/** + * Format and print out errors for a list of files. + * @param {Object[]} filesWithErrors - each element is an object containing a path, and errors associated with that path. + */ +const printErrors = (filesWithErrors) => { + for(const f of filesWithErrors){ + let outputMessage = `\nError: ${removeCWDPrefix(f.path)}`; + for(const e of f.errors){ + outputMessage += `\n\t ${e.message}`; + } + console.log(outputMessage); + } + console.log(''); +} + const main = () => { const filePaths = getPackFilePaths(process.cwd()).sort(); const files = filePaths.map(readPackFile); const filesWithErrors = files.map(validateFile).filter(file => file.errors.length > 0); - for (const f of filesWithErrors) { - console.log(`\nError: ${removeCWDPrefix(f.path)}`); - for (const e of f.errors) { - console.log(`\t ${e.message}`); - } - } - console.log(''); + printErrors(filesWithErrors); if (filesWithErrors.length > 0) { process.exit(1); @@ -137,4 +153,3 @@ if (require.main === module) { } module.exports = { validateFile, convertErrors }; - From 6439ae41f31ac76bbaf4fd32cd68af8c18b83988 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:45:10 -0700 Subject: [PATCH 06/15] chore: added test for printErrors --- utils/__tests__/validate_packs.test.js | 50 +++++++++++++++++++++++++- utils/validate_packs.js | 3 +- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/utils/__tests__/validate_packs.test.js b/utils/__tests__/validate_packs.test.js index 4063b5bcf2..9d1cbad6e2 100644 --- a/utils/__tests__/validate_packs.test.js +++ b/utils/__tests__/validate_packs.test.js @@ -1,4 +1,8 @@ -const { validateFile, convertErrors } = require('../validate_packs'); +const { + validateFile, + convertErrors, + printErrors, +} = require('../validate_packs'); jest.spyOn(global.console, 'log').mockImplementation(() => {}); jest.spyOn(global.console, 'error').mockImplementation(() => {}); @@ -385,3 +389,47 @@ describe('test converErrors', () => { ]); }); }); + +describe('test printErrors', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + test('displays correct output', () => { + const mockFileErrors = [ + { + path: `${process.cwd()}/fake_path`, + errors: [ + { + message: `'/type' must be equal to one of the allowed values: ["STATIC","BASELINE","OUTLIER"]`, + }, + { + message: "must have required property 'name'", + }, + ], + }, + { + path: `${process.cwd()}/fake_path_2`, + errors: [ + { + message: `'/type' must be equal to one of the allowed values: ["RED","BLUE","GREEN"]`, + }, + { + message: "must have required property 'ball'", + }, + ], + }, + ]; + + printErrors(mockFileErrors); + + expect(global.console.log).toHaveBeenNthCalledWith( + 1, + `\nError: fake_path\n\t '/type' must be equal to one of the allowed values: [\"STATIC\",\"BASELINE\",\"OUTLIER\"]\n\t must have required property 'name'` + ); + expect(global.console.log).toHaveBeenNthCalledWith( + 2, + `\nError: fake_path_2\n\t '/type' must be equal to one of the allowed values: [\"RED\",\"BLUE\",\"GREEN\"]\n\t must have required property 'ball'` + ); + }); +}); diff --git a/utils/validate_packs.js b/utils/validate_packs.js index 8e9aa691a4..2ac5f75ded 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -74,7 +74,6 @@ const validateFile = (file) => { const filePath = file.path; let errors = []; - console.log(`Validating ${removeCWDPrefix(filePath)}`); switch(true) { case(filePath.includes('/alerts/')): // validate using alert schema errors = validateAgainstSchema(file.contents[0], alertSchema); @@ -152,4 +151,4 @@ if (require.main === module) { main(); } -module.exports = { validateFile, convertErrors }; +module.exports = { validateFile, convertErrors, printErrors }; From 276f19726614eb70c5b06935a641e47e917eeec3 Mon Sep 17 00:00:00 2001 From: Ruairi Douglas Date: Mon, 28 Jun 2021 19:47:46 +0100 Subject: [PATCH 07/15] Update utils/validate_packs.js --- utils/validate_packs.js | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/validate_packs.js b/utils/validate_packs.js index 2ac5f75ded..85ae71c313 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -28,7 +28,6 @@ const EXCLUDED_DIRECTORY_PATTERNS = [ */ const convertErrors = (ajvErrors) => { const errors = ajvErrors.map((e) => { - console.log(e); let message = ''; switch(true) { From b93c4c7bb1f98c9e27eaec0c61dbe19dfb7ea2bf Mon Sep 17 00:00:00 2001 From: Ruairi Douglas Date: Mon, 28 Jun 2021 19:47:58 +0100 Subject: [PATCH 08/15] Update utils/__tests__/validate_packs.test.js --- utils/__tests__/validate_packs.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/__tests__/validate_packs.test.js b/utils/__tests__/validate_packs.test.js index 9d1cbad6e2..63baf90082 100644 --- a/utils/__tests__/validate_packs.test.js +++ b/utils/__tests__/validate_packs.test.js @@ -298,7 +298,7 @@ describe('test validateFile', () => { }); }); -describe('test converErrors', () => { +describe('test convertErrors', () => { test('converts enum error message', () => { const mockAjvErrors = [ { From 19bb7db929b0035435a3b224ba544e315b6f60d7 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Mon, 28 Jun 2021 13:35:08 -0700 Subject: [PATCH 09/15] chore: prettier format --- utils/validate_packs.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/utils/validate_packs.js b/utils/validate_packs.js index 6280cb74fc..e3e4850fb8 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -24,15 +24,14 @@ const EXCLUDED_DIRECTORY_PATTERNS = ['node_modules/**', 'utils/**', '*']; */ const convertErrors = (ajvErrors) => { const errors = ajvErrors.map((e) => { - let message = ''; - switch(true) { - case (e.keyword === 'enum'): + switch (true) { + case e.keyword === 'enum': message = `'${e.instancePath}' ${e.message}: ${JSON.stringify( e.params.allowedValues )}`; return { message }; - case (e.keyword === 'required' && e.instancePath != ''): + case e.keyword === 'required' && e.instancePath != '': message = `'${e.instancePath}' ${e.message}`; return { message }; default: @@ -43,12 +42,12 @@ const convertErrors = (ajvErrors) => { return errors; }; -/** -* Validates an object against a JSON schema -* @param {Object} content - The object to validate -* @param {Object} schema - Json schema used for validation. -* @returns {Object[]} An array of any errors found -*/ +/** + * Validates an object against a JSON schema + * @param {Object} content - The object to validate + * @param {Object} schema - Json schema used for validation. + * @returns {Object[]} An array of any errors found + */ const validateAgainstSchema = (content, schema) => { const validate = ajv.compile(schema); const valid = validate(content); @@ -121,15 +120,15 @@ const getPackFilePaths = (basePath) => { * @param {Object[]} filesWithErrors - each element is an object containing a path, and errors associated with that path. */ const printErrors = (filesWithErrors) => { - for(const f of filesWithErrors){ + for (const f of filesWithErrors) { let outputMessage = `\nError: ${removeCWDPrefix(f.path)}`; - for(const e of f.errors){ + for (const e of f.errors) { outputMessage += `\n\t ${e.message}`; } console.log(outputMessage); } console.log(''); -} +}; const main = () => { const filePaths = getPackFilePaths(process.cwd()).sort(); From 6f09d53ee486e5763d25d20dd49fed6c1c9a6891 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Mon, 28 Jun 2021 13:40:30 -0700 Subject: [PATCH 10/15] chore: add comment explaining require.main --- utils/validate_packs.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/utils/validate_packs.js b/utils/validate_packs.js index e3e4850fb8..beade45b39 100644 --- a/utils/validate_packs.js +++ b/utils/validate_packs.js @@ -145,6 +145,11 @@ const main = () => { } }; +/** + * This allows us to check if the script was invoked directly from the command line, i.e 'node validate_packs.js', or if it was imported. + * This would be true if this was used in one of our GitHub workflows, but false when imported for use in a test. + * See here: https://nodejs.org/docs/latest/api/modules.html#modules_accessing_the_main_module + */ if (require.main === module) { main(); } From 843d77f7672c4e3dec71f3cd327ca7b8d5d7ed9f Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Mon, 28 Jun 2021 14:27:41 -0700 Subject: [PATCH 11/15] chore: add prettier script, dependency --- package.json | 6 ++++-- yarn.lock | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 9fa767f02c..90085f0066 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,8 @@ "prepare": "husky install", "validate-packs": "node utils/validate_packs.js", "test": "jest", - "check-for-matching-names": "node utils/check_pack_uniqueness.js" + "check-for-matching-names": "node utils/check_pack_uniqueness.js", + "format": "prettier --write \"utils/**/*.js\"" }, "devDependencies": { "@commitlint/cli": "^12.1.4", @@ -17,6 +18,7 @@ "glob": "^7.1.7", "husky": "^6.0.0", "jest": "^27.0.5", - "js-yaml": "^4.1.0" + "js-yaml": "^4.1.0", + "prettier": "2.3.2" } } diff --git a/yarn.lock b/yarn.lock index 6597bff8dc..8fb14c9dcc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2601,6 +2601,11 @@ prelude-ls@~1.1.2: resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54" integrity sha1-IZMqVJ9eUv/ZqCf1cOBL5iqX2lQ= +prettier@2.3.2: + version "2.3.2" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.3.2.tgz#ef280a05ec253712e486233db5c6f23441e7342d" + integrity sha512-lnJzDfJ66zkMy58OL5/NY5zp70S7Nz6KqcKkXYzn2tMVrNxvbqaBpg7H3qHaLxCJ5lNMsGuM8+ohS7cZrthdLQ== + pretty-format@^27.0.2: version "27.0.2" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-27.0.2.tgz#9283ff8c4f581b186b2d4da461617143dca478a4" From b72ba1cacb3f5eb9f3661ebfb61da5b01b135b6f Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Mon, 28 Jun 2021 14:27:58 -0700 Subject: [PATCH 12/15] chore: update documentation --- CONTRIBUTING.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fdfc2de7fc..8e8b29b94b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -45,6 +45,12 @@ Before submitting an Issue, please search for similar ones in the 3. Ensure that all status checks are passing. 4. You may merge the Pull Request in once you have the sign-off of one other developer, or if you do not have permission to do that, you may request the reviewer to merge it for you. +### Status Checks + +#### Schema Validation + +One of the required checks is ensuring that submitted packs and their components are valid. To be valid, a configuration file needs to have all required fields filled out, with all fields having appropriate values. choreThe schemas for those checks live in [utils/schemas](../utils/schemas). + ## Using Conventional Commits Please help the maintainers by leveraging the following [conventional commit](https://www.conventionalcommits.org/en/v1.0.0/) From 67c4c94b85325983050b31aabeeecf9b23499d5d Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Mon, 28 Jun 2021 14:29:04 -0700 Subject: [PATCH 13/15] chore: update documentation --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8e8b29b94b..f3fa8d20cb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,7 +49,7 @@ Before submitting an Issue, please search for similar ones in the #### Schema Validation -One of the required checks is ensuring that submitted packs and their components are valid. To be valid, a configuration file needs to have all required fields filled out, with all fields having appropriate values. choreThe schemas for those checks live in [utils/schemas](../utils/schemas). +One of the required checks is ensuring that submitted packs and their components are valid. To be valid, a configuration file needs to have all required fields filled out, with all fields having appropriate values. The schemas for those checks live in [utils/schemas](./utils/schemas). ## Using Conventional Commits From 00b3ba55658e514a9dfac6be658167827bc22311 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Tue, 29 Jun 2021 09:47:05 -0700 Subject: [PATCH 14/15] chore: move prettier config files --- .prettierignore => utils/.prettierignore | 0 .prettierrc.js => utils/.prettierrc.js | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename .prettierignore => utils/.prettierignore (100%) rename .prettierrc.js => utils/.prettierrc.js (97%) diff --git a/.prettierignore b/utils/.prettierignore similarity index 100% rename from .prettierignore rename to utils/.prettierignore diff --git a/.prettierrc.js b/utils/.prettierrc.js similarity index 97% rename from .prettierrc.js rename to utils/.prettierrc.js index 7b7fcc0d10..94ccc501ce 100644 --- a/.prettierrc.js +++ b/utils/.prettierrc.js @@ -5,4 +5,4 @@ module.exports = { semi: true, singleQuote: true, arrowParens: 'always', -}; +} From 653c6187ea2c9c975022ff22c594a012e44d6bf9 Mon Sep 17 00:00:00 2001 From: Kris Kenney <70179215+nr-kkenney@users.noreply.github.com> Date: Tue, 29 Jun 2021 10:04:33 -0700 Subject: [PATCH 15/15] chore: add semicolon --- utils/.prettierrc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/.prettierrc.js b/utils/.prettierrc.js index 94ccc501ce..7b7fcc0d10 100644 --- a/utils/.prettierrc.js +++ b/utils/.prettierrc.js @@ -5,4 +5,4 @@ module.exports = { semi: true, singleQuote: true, arrowParens: 'always', -} +};