Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: log errors when validators throw #41

Merged
merged 1 commit into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions lib/mergeDeep.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const NAME_USERNAME_PROPERTY = item => NAME_FIELDS.find(prop => Object.prototype
const GET_NAME_USERNAME_PROPERTY = item => { if (NAME_USERNAME_PROPERTY(item)) return item[NAME_USERNAME_PROPERTY(item)] }

class MergeDeep {
constructor (log, github, ignorableFields, configvalidators = {}, overridevalidators = {}) {
constructor (log, github, ignorableFields) {
this.log = log
this.github = github
this.ignorableFields = ignorableFields
Expand Down Expand Up @@ -269,20 +269,30 @@ class MergeDeep {
return
}
if (this.overridevalidators[key]) {
// this.log.debug(`Calling overridevalidator for key ${key} `)
if (!this.overridevalidators[key].canOverride(baseconfig, overrideconfig, this.github)) {
this.log.error(`Error in calling overridevalidator for key ${key} ${this.overridevalidators[key].error}`)
throw new Error(this.overridevalidators[key].error)
this.log.debug(`MergeDeep calling overridevalidator for key ${key} `)
try {
const canOverride = this.overridevalidators[key].canOverride(baseconfig, overrideconfig, this.github)
if (!canOverride) {
throw new Error(this.overridevalidators[key].error)
}
} catch (error) {
this.log.error(`MergeDeep Error in calling overridevalidator for key ${key} ${error}`)
throw error
}
}
}

validateConfig (key, baseconfig) {
if (this.configvalidators[key]) {
// this.log.debug(`Calling configvalidator for key ${key} `)
if (!this.configvalidators[key].isValid(baseconfig, this.github)) {
this.log.error(`Error in calling configvalidator for key ${key} ${this.configvalidators[key].error}`)
throw new Error(this.configvalidators[key].error)
this.log.debug(`MergeDeep calling configvalidator for key ${key} `)
try {
const isValid = this.configvalidators[key].isValid(baseconfig, this.github)
if (!isValid) {
throw new Error(this.configvalidators[key].error)
}
} catch (error) {
this.log.error(`MergeDeep Error in calling configvalidator for key ${key} ${error}`)
throw error
}
}
}
Expand Down
22 changes: 16 additions & 6 deletions lib/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,17 +443,27 @@ ${stripAllWhitespace(JSON.stringify(quietResults, null, 2))}
const configValidator = this.configvalidators[section]
if (configValidator) {
this.log.debug(`Calling configvalidator for key ${section} `)
if (!configValidator.isValid(overrideConfig, this.github)) {
this.log.error(`Error in calling configvalidator for key ${section} ${configValidator.error}`)
throw new Error(configValidator.error)
try {
const isValid = configValidator.isValid(overrideConfig, this.github)
if (!isValid) {
throw new Error(configValidator.error)
}
} catch (error) {
this.log.error(`Error in calling configvalidator for key ${section} ${error}`)
throw error
}
}
const overridevalidator = this.overridevalidators[section]
if (overridevalidator) {
this.log.debug(`Calling overridevalidator for key ${section} `)
if (!overridevalidator.canOverride(baseConfig, overrideConfig, this.github)) {
this.log.error(`Error in calling overridevalidator for key ${section} ${overridevalidator.error}`)
throw new Error(overridevalidator.error)
try {
const canOverride = overridevalidator.canOverride(baseConfig, overrideConfig, this.github)
if (!canOverride) {
throw new Error(overridevalidator.error)
}
} catch (error) {
this.log.error(`Error in calling overridevalidator for key ${section} ${error}`)
throw error
}
}
}
Expand Down
54 changes: 52 additions & 2 deletions test/unit/lib/validator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@
const DeploymentConfig = require('../../../lib/deploymentConfig')
const MergeDeep = require('../../../lib/mergeDeep')
const YAML = require('js-yaml')
const log = require('pino')('test.log')

const log = { debug: jest.fn(), error: jest.fn() }

describe('Validator Tests', () => {
beforeEach(() => {
jest.clearAllMocks()
})

it('Branch override validator test', () => {
const overrideMock = jest.fn((baseconfig, overrideconfig) => {
if (baseconfig.protection.required_pull_request_reviews.required_approving_review_count && overrideconfig.protection.required_pull_request_reviews.required_approving_review_count) {
Expand Down Expand Up @@ -113,7 +118,7 @@ describe('Validator Tests', () => {

try {
const ignorableFields = []
const mergeDeep = new MergeDeep(log, ignorableFields)
const mergeDeep = new MergeDeep(log, {}, ignorableFields)
mergeDeep.mergeDeep(baseconfig, overrideconfig)
} catch (err) {
expect(err).toBeDefined()
Expand Down Expand Up @@ -176,4 +181,49 @@ describe('Validator Tests', () => {
}
expect(configMock.mock.calls.length).toBe(1)
})

it('override validator supports custom messages', () => {
const overrideMock = jest.fn(() => {
throw new Error('Custom message')
})

const configMock = jest.fn((baseconfig) => {
console.log(`Branch config validator, baseconfig ${baseconfig}`)
return false
})
DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } }
DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } }

const overrideconfig = {
branches: {}
}
const baseconfig = {
branches: {}
}

const mergeDeep = new MergeDeep(log, {}, [])
expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Custom message')
expect(log.error).toHaveBeenCalledWith(`MergeDeep Error in calling overridevalidator for key branches Error: Custom message`)
})

it('config validator supports custom messages', () => {
const overrideMock = jest.fn(() => true)

const configMock = jest.fn(() => {
throw new Error('Custom message')
})
DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } }
DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } }

const overrideconfig = {
branches: {}
}
const baseconfig = {
branches: {}
}

const mergeDeep = new MergeDeep(log, {}, [])
expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('Custom message')
expect(log.error).toHaveBeenCalledWith(`MergeDeep Error in calling configvalidator for key branches Error: Custom message`)
})
})
Loading