From 78015a9279ec682fecfe0ebacee25480f7d96755 Mon Sep 17 00:00:00 2001 From: Stephen Collings Date: Mon, 20 May 2024 13:49:15 +0100 Subject: [PATCH 1/2] test: fix validator tests never fail --- test/unit/lib/validator.test.js | 71 ++++++++------------------------- 1 file changed, 17 insertions(+), 54 deletions(-) diff --git a/test/unit/lib/validator.test.js b/test/unit/lib/validator.test.js index 5283ab0a..d0d5667f 100644 --- a/test/unit/lib/validator.test.js +++ b/test/unit/lib/validator.test.js @@ -16,15 +16,9 @@ describe('Validator Tests', () => { return overrideconfig.protection.required_pull_request_reviews.required_approving_review_count >= baseconfig.protection.required_pull_request_reviews.required_approving_review_count } return true - - // console.log(`Branch override validator, baseconfig ${baseconfig} overrideconfig ${overrideconfig}`) - // return false }) - const configMock = jest.fn((baseconfig) => { - console.log(`Branch config validator, baseconfig ${baseconfig}`) - return false - }) + const configMock = jest.fn((baseconfig) => false) DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } } DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } } @@ -58,30 +52,17 @@ describe('Validator Tests', () => { enforce_admins: false `) - try { - const ignorableFields = [] - const mergeDeep = new MergeDeep(log, {}, ignorableFields) - mergeDeep.mergeDeep(baseconfig, overrideconfig) - // const merged = mergeDeep.mergeDeep(baseconfig, overrideconfig) - // expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('you are using the wrong JDK'); - } catch (err) { - expect(err).toBeDefined() - console.log(JSON.stringify(err)) - expect(err).toEqual(Error('Branch overrideValidators.error')) - } + const ignorableFields = [] + const mergeDeep = new MergeDeep(log, {}, ignorableFields) + + expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Branch overrideValidators.error') expect(overrideMock.mock.calls.length).toBe(1) }) it('Repository override validator test', () => { - const overrideMock = jest.fn((baseconfig, overrideconfig) => { - console.log(`Repo override validator, baseconfig ${baseconfig} overrideconfig ${overrideconfig}`) - return false - }) + const overrideMock = jest.fn(() => false) - const configMock = jest.fn((baseconfig) => { - console.log(`Repo config validator, baseconfig ${baseconfig}`) - return false - }) + const configMock = jest.fn(() => false) DeploymentConfig.overridevalidators = { repository: { canOverride: overrideMock, error: 'Repo overrideValidators.error' } } DeploymentConfig.configvalidators = { repository: { isValid: configMock, error: 'Repo configValidators.error' } } @@ -116,27 +97,16 @@ describe('Validator Tests', () => { - newone `) - try { - const ignorableFields = [] - const mergeDeep = new MergeDeep(log, {}, ignorableFields) - mergeDeep.mergeDeep(baseconfig, overrideconfig) - } catch (err) { - expect(err).toBeDefined() - expect(err).toEqual(Error('Repo overrideValidators.error')) - } + const ignorableFields = [] + const mergeDeep = new MergeDeep(log, {}, ignorableFields) + expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Repo overrideValidators.error') expect(overrideMock.mock.calls.length).toBe(1) }) it('Repository config validator test', () => { - const overrideMock = jest.fn((baseconfig, overrideconfig) => { - console.log(`Repo override validator, baseconfig ${baseconfig} overrideconfig ${overrideconfig}`) - return true - }) + const overrideMock = jest.fn(() => true) - const configMock = jest.fn((baseconfig) => { - console.log(`Repo config validator, baseconfig ${baseconfig}`) - return false - }) + const configMock = jest.fn(() => false) DeploymentConfig.overridevalidators = { repository: { canOverride: overrideMock, error: 'Repo overrideValidators.error' } } DeploymentConfig.configvalidators = { repository: { isValid: configMock, error: 'Repo configValidators.error' } } @@ -171,14 +141,10 @@ describe('Validator Tests', () => { - newone `) - try { - const ignorableFields = [] - const mergeDeep = new MergeDeep(log, ignorableFields) - mergeDeep.mergeDeep(baseconfig, overrideconfig) - } catch (err) { - expect(err).toBeDefined() - expect(err).toEqual(Error('Repo configValidators.error')) - } + const ignorableFields = [] + const mergeDeep = new MergeDeep(log, {}, ignorableFields) + + expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Repo configValidators.error') expect(configMock.mock.calls.length).toBe(1) }) @@ -187,10 +153,7 @@ describe('Validator Tests', () => { throw new Error('Custom message') }) - const configMock = jest.fn((baseconfig) => { - console.log(`Branch config validator, baseconfig ${baseconfig}`) - return false - }) + const configMock = jest.fn(() => false) DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } } DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } } From fc9bfee0fec0938db80708ba58d8d7e7d647b717 Mon Sep 17 00:00:00 2001 From: Stephen Collings Date: Mon, 20 May 2024 14:11:51 +0100 Subject: [PATCH 2/2] docs: Specific validation messages --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index ca460013..d43633b4 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,23 @@ overridevalidators: A sample of `deployment-settings` file is found [here](docs/sample-settings/sample-deployment-settings.yml). +#### Specific validation messages + +Validators can also throw errors rather than returning `false` in order to provide specific messages: + +```yaml +configvalidators: + - plugin: collaborators + error: | + `Invalid collaborators config` + script: | + if (baseconfig.permission == 'admin') { + throw new Error(`collaborator permission: admin not allowed`) + } + + return true +``` + ### Performance When there are 1000s of repos to be managed -- and there is a global settings change -- safe-settings will have to work efficiently and only make the necessary API calls.