Skip to content

Commit

Permalink
Allowing specified unknown keys in plugin validator.
Browse files Browse the repository at this point in the history
  • Loading branch information
nataliecarey committed Aug 25, 2023
1 parent 31a76a3 commit 9c42d3a
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

## New features

- [#2314: Plugin validator allows the user to specify unknown keys](https://github.com/alphagov/govuk-prototype-kit/pull/2314)

## 13.12.2

### Fixes
Expand Down
4 changes: 4 additions & 0 deletions __tests__/utils/mock-file-system.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// core dependencies
const path = require('path')
const fs = require('fs')
const fse = require('fs-extra')

function throwNotFound (filePath) {
const err = new Error(`ENOENT: no such file or directory, open '${filePath}'`)
Expand Down Expand Up @@ -156,6 +157,8 @@ function mockFileSystem (rootPath) {
jest.spyOn(fs, 'existsSync').mockImplementation(existsImplementation)
jest.spyOn(fs, 'lstatSync').mockImplementation(lstatImplementation)
jest.spyOn(fs, 'readdirSync').mockImplementation(readdirImplementation)
jest.spyOn(fs, 'existsSync').mockImplementation(existsImplementation)
jest.spyOn(fse, 'exists').mockImplementation(promiseWrap(existsImplementation))
jest.spyOn(fs.promises, 'readFile').mockImplementation(promiseWrap(readFileImplementation))
jest.spyOn(fs.promises, 'writeFile').mockImplementation(promiseWrap(writeFileImplementation))
jest.spyOn(fs.promises, 'lstat').mockImplementation(promiseWrap(lstatImplementation))
Expand All @@ -174,6 +177,7 @@ function mockFileSystem (rootPath) {
spiesToTearDown.push(fs.writeFileSync)
spiesToTearDown.push(fs.existsSync)
spiesToTearDown.push(fs.lstatSync)
spiesToTearDown.push(fse.exists)
spiesToTearDown.push(fs.promises.readFile)
spiesToTearDown.push(fs.promises.writeFile)
spiesToTearDown.push(fs.promises.lstat)
Expand Down
2 changes: 1 addition & 1 deletion bin/cli
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ function runServe () {
}

async function runValidatePlugin () {
return validatePlugin(getInstallLocation())
return validatePlugin(getInstallLocation(), argv)
}

;(async () => {
Expand Down
18 changes: 11 additions & 7 deletions lib/plugins/plugin-validator.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const fse = require('fs-extra')
const fs = require('fs')
const path = require('path')

const fse = require('fs-extra')
const ansiColors = require('ansi-colors')

const errors = []
Expand Down Expand Up @@ -48,12 +50,14 @@ function reportInvalidKeys (invalidKeys) {
errors.push(`The following invalid keys exist in your config: ${invalidKeys}`)
}

function validateConfigKeys (pluginConfig) {
function validateConfigKeys (pluginConfig, argv) {
console.log('Config file exists, validating contents.')
const keysToAllowThrough = argv?.options?.keysToIgnoreIfUnknown || ''
const allowedKeys = knownKeys.concat(keysToAllowThrough.split(',').filter(key => !!key))
const invalidKeys = []

const validKeysPluginConfig = Object.fromEntries(Object.entries(pluginConfig).filter(([key]) => {
if (knownKeys.includes(key)) {
if (allowedKeys.includes(key)) {
return true
}
invalidKeys.push(key)
Expand Down Expand Up @@ -184,7 +188,7 @@ function validateConfigurationValues (pluginConfig, executionPath) {
checkNunjucksMacroExists(executionPath, configEntry.importFrom, pluginConfig.nunjucksPaths)
} else if (typeof configEntry === 'string' && configEntry[0] === '/') {
checkPathExists(executionPath, configEntry, key)
} else {
} else if (knownKeys.includes(key)) {
// Find the path for the ones that can be objects
const invalidPath = (key === 'templates' || (key === 'scripts' && configEntry.path !== undefined))
? configEntry.path
Expand All @@ -198,14 +202,14 @@ function validateConfigurationValues (pluginConfig, executionPath) {
})
}

async function validatePlugin (executionPath) {
async function validatePlugin (executionPath, argv) {
console.log()
const configPath = path.join(executionPath, 'govuk-prototype-kit.config.json')
await fse.exists(configPath).then(exists => {
if (exists) {
let pluginConfig
try {
pluginConfig = JSON.parse(fse.readFileSync(configPath).toString())
pluginConfig = JSON.parse(fs.readFileSync(configPath, 'utf8'))
} catch (error) {
// Catch any syntax errors in the config
errors.push('Your govuk-prototype-kit.config.json file is not valid json.')
Expand All @@ -225,7 +229,7 @@ async function validatePlugin (executionPath) {
errors.push(`There are no contents in your govuk-prototype.config file${caveat}!`)
} else {
// Perform the rest of the checks if the config file has contents
const validKeysPluginConfig = validateConfigKeys(pluginConfig)
const validKeysPluginConfig = validateConfigKeys(pluginConfig, argv)
validateConfigurationValues(validKeysPluginConfig, executionPath)
}
}
Expand Down
76 changes: 75 additions & 1 deletion lib/plugins/plugin-validator.spec.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,84 @@
const { validatePluginDependency, clearErrors, getErrors, validateMeta } = require('./plugin-validator')
const { validatePluginDependency, clearErrors, getErrors, validateMeta, validatePlugin } = require('./plugin-validator')
const { mockFileSystem } = require('../../__tests__/utils/mock-file-system')
const path = require('path')
const ansiColors = require('ansi-colors')

describe('plugin-validator', () => {
let testScope = {}

const phaseOneSuccessMessages = [
'Config file exists, validating contents.',
'Validating whether config paths meet criteria.'
]
const validSuccessMessages = phaseOneSuccessMessages.concat([ansiColors.green('The plugin config is valid.')])

beforeEach(() => {
const mockFileSystemRoot = path.join(process.cwd(), 'example-plugin')
testScope = {
mockFileSystemRoot,
fileSystem: mockFileSystem(mockFileSystemRoot),
stdLogs: [],
errLogs: []
}
jest.spyOn(console, 'log').mockImplementation((message, ...args) => {
if (args.length > 0) {
testScope.stdLogs.push({ message, args })
} else if (message !== undefined) {
testScope.stdLogs.push(message)
}
})
jest.spyOn(console, 'error').mockImplementation((message, ...args) => {
if (args.length > 0) {
testScope.errLogs.push({ message, args })
} else if (message !== undefined) {
testScope.errLogs.push(message)
}
})
testScope.fileSystem.setupSpies()
clearErrors()
})

afterEach(() => {
jest.clearAllMocks()
})

describe('unexpected keys', () => {
it('should fail when a key is unknown', async () => {
testScope.fileSystem.writeFile(['govuk-prototype-kit.config.json'], JSON.stringify({
myExample: ['abc']
}))
await validatePlugin(testScope.mockFileSystemRoot)
expect(testScope.errLogs).toEqual([ansiColors.red('Error: The following invalid keys exist in your config: myExample')])
expect(testScope.stdLogs).toEqual(phaseOneSuccessMessages)
})
it('should allow a single unknown key when specified', async () => {
testScope.fileSystem.writeFile(['govuk-prototype-kit.config.json'], JSON.stringify({
myExample: ['abc']
}))
await validatePlugin(testScope.mockFileSystemRoot, {
options: {
keysToIgnoreIfUnknown: 'myExample'
}
})
expect(testScope.errLogs).toEqual([])
expect(testScope.stdLogs).toEqual(validSuccessMessages)
})
it('should allow multiple unknown keys when specified', async () => {
testScope.fileSystem.writeFile(['govuk-prototype-kit.config.json'], JSON.stringify({
myExample: ['abc'],
anotherExample: ['def'],
third: ['ghi']
}))
await validatePlugin(testScope.mockFileSystemRoot, {
options: {
keysToIgnoreIfUnknown: 'myExample,anotherExample,third'
}
})
expect(testScope.errLogs).toEqual([])
expect(testScope.stdLogs).toEqual(validSuccessMessages)
})
})

describe('validatePluginDependency', () => {
it('should be valid with a string', () => {
validatePluginDependency('pluginDependencies', 'test-package')
Expand Down

0 comments on commit 9c42d3a

Please sign in to comment.