Skip to content

Commit

Permalink
feat(features): display dupe acs in features and mystery ones (#117)
Browse files Browse the repository at this point in the history
* feat(features): display dupe acs in features and mystery ones
* feat(features): add check-features command
* feat(features): add verbose output option
  • Loading branch information
edd authored Aug 11, 2023
1 parent c686903 commit 23bcc62
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 41 deletions.
53 changes: 38 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ All of the globs below are relatively simple - check out [globs primer](https://
|-----------------|----------|--------------------------------------|----------------------|
| `--specs` | glob | specs to pull AC codes from | `{specs/**/*.md}` |
| `--ignore` | glob | glob of files not to check for codes | `specs/0001-spec.md` |
| `--show-branches` | boolean | Show git branches for subfolders of the current folder | - |
| `--show-branches` | boolean | Show git branches for subfolders of the current folder | - |

### check-codes example
```bash
# Use node
npx @vegaprotocol/approbation check-filenames --specs="./specs/protocol/**/*.{md,ipynb}" --show-branches
npx @vegaprotocol/approbation check-filenames --specs="./specs/protocol/**/*.{md,ipynb}" --show-branches

# Or run the docker image
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest check-codes --specs="/run/specs/protocol/**/*.{md,ipynb}" --show-branches
Expand All @@ -41,7 +41,7 @@ docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest check-codes
|-----------------|----------|--------------------------------------|----------------------|
| `--specs` | glob | specs to pull AC codes from | `{specs/**/*.md}` |
| `--ignore` | glob | glob of files not to check for codes | `specs/0001-spec.md` |
| `--show-branches` | boolean | Show git branches for subfolders of the current folder | - |
| `--show-branches` | boolean | Show git branches for subfolders of the current folder | - |

### check-filenames example
```bash
Expand All @@ -51,11 +51,34 @@ npx @vegaprotocol/approbation check-filenames codes --specs="./specs/protocol/**
# Or run the docker image
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest check-filenames codes --specs="/run/specs/protocol/**/*.{md,ipynb}" --tests="/run/MultisigControl/test/*.js" --ignore="/run/specs/protocol/{0001-*,0002-*,0004-
```
## check-features
> Validates a feature file
**Arguments**
| **Parameter** | **Type** | **Description** | **Example** |
|-----------------|----------|--------------------------------------|----------------------|
| `--specs` | glob | specs to pull AC codes from | `{specs/**/*.md}` |
| `--ignore` | glob | glob of files not to check for codes | `specs/0001-spec.md` |
| `--features` | string | JSON file that contains features mappings for specs | `specs/features.json` |
| `--verbose` | boolean | Also list all acs not in a feature | - |
### check-features example
```bash
# Use node
npx @vegaprotocol/approbation check-features
--specs="{./specs/protocol/**/*.{md,ipynb},./specs/non-protocol-specs/**/*.{md,ipynb}}"
--ignore="{./spec-internal/protocol/0060*,./specs/non-protocol-specs/{0001-NP*,0002-NP*,0004-NP*,0006-NP*,0007-NP*,0008-NP*,0010-NP*}}"
--features="specs/protocol/features.json"
# Or run the docker image
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest check-features --specs="/run/specs/protocol/**/*.{md,ipynb}"
--ignore="{./spec-internal/protocol/0060*,./specs/non-protocol-specs/{0001-NP*,0002-NP*,0004-NP*,0006-NP*,0007-NP*,0008-NP*,0010-NP*}}"
--features="specs/protocol/features.json"
```
## check-references
> Coverage statistics for acceptance criteria
**Arguments**
| **Parameter** | **Type** | **Description** | **Example** |
| ------------------- | -------- | --------------------------------------------------------------------------- | ------------------------- |
Expand All @@ -82,7 +105,7 @@ npx github:vegaprotocol/approbation@latest check-references --specs="./specs/pro
# Or run the docker image
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest check-references --specs="/run/specs/protocol/**/*.{md,ipynb}" --tests="/run/MultisigControl/test/*.js" --ignore="/run/specs/protocol/{0001-*}" --categories="/run/specs/protocol/categories.json" --show-branches --show-mystery --output-csv --output="/run/results/"
```
## next-filename
> Suggests what file sequence number to use next, given a list of spec files
Expand All @@ -91,16 +114,16 @@ docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest check-refere
|-----------------|----------|--------------------------------------|----------------------|
| `--specs` | glob | specs to pull AC codes from | `{specs/**/*.md}` |
| `--ignore` | glob | glob of files not to include | `specs/0001-spec.md` |
| `--verbose` | boolean | Display extra output | - |
| `--verbose` | boolean | Display extra output | - |
In the case of three specs, ['001...', '002...', '003...'] this would suggest '004'. However if '002' didn't exist, it would indicate that '002' is available, as well as '004'.
### next-filename example
```bash
# Use node
npx @vegaprotocol/approbation next-filename --specs="./specs/protocol/**/*.{md,ipynb}"
npx @vegaprotocol/approbation next-filename --specs="./specs/protocol/**/*.{md,ipynb}"
# Or run the docker image
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest next-filename --specs="/run/specs/protocol/**/*.{md,ipynb}"
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest next-filename --specs="/run/specs/protocol/**/*.{md,ipynb}"
```
## next-code
Expand All @@ -111,23 +134,23 @@ docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest next-filenam
|-----------------|----------|--------------------------------------|----------------------|
| `--specs` | glob | spec to pull AC codes from. Should only match one file. | `{specs/**/0001-SPEC-spec.md}` |
| `--ignore` | glob | glob of files not to include | `specs/0001-spec.md` |
| `--verbose` | boolean | Display extra output | - |
| `--verbose` | boolean | Display extra output | - |
Like `next-filename`, `next-code` will suggest the lowest available code in the sequence (e.g. if there is `0001-SPEC-001` and `0001-SPEC-003`), it will suggest both `0001-SPEC-002` and `000-SPEC-004`. If using the lowest available code, ensure it isn't already referenced by any tests (i.e. isn't listed as a 'Mystery Criteria' by `check-references`).
### next-code example
```bash
# Use node
npx @vegaprotocol/approbation next-filename --specs="./specs/protocol/**/*.{md,ipynb}"
npx @vegaprotocol/approbation next-filename --specs="./specs/protocol/**/*.{md,ipynb}"
# Or run the docker image
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest next-filename --specs="/run/specs/protocol/**/*.{md,ipynb}"
docker run -v "$(pwd):/run" ghcr.io/vegaprotocol/approbation:latest next-filename --specs="/run/specs/protocol/**/*.{md,ipynb}"
```
# Background
Each [protocol specification](https://github.com/vegaprotocol/specs) receives a sequence number when it is merged in to master.
This sequence number is a 0-padded integer, strictly 1 greater than the last merged
Each [protocol specification](https://github.com/vegaprotocol/specs) receives a sequence number when it is merged in to master.
This sequence number is a 0-padded integer, strictly 1 greater than the last merged
specification. The sequence number is the start of the filename, with the end result
that the `./protocol/` folder lists files in the order they were created.
Expand Down Expand Up @@ -161,11 +184,11 @@ reasonable.
5. Merge!
## How to reference acceptance criteria in a spec.
These are more *convention* than a rule, but following these steps will ensure that the scripts in
These are more *convention* than a rule, but following these steps will ensure that the scripts in
this folder pick up your references.
1. When you are writing your feature, take a look at the acceptance criteria.
2. If you're addressing one, reference it at the end of the Feature name, for example if you are
2. If you're addressing one, reference it at the end of the Feature name, for example if you are
writing a test that covers `0008-SYSA-001`, call the feature `Verify blah (0008-SYSA-001)`
3. If it covers more than one feature, add it inside the same brackets: `Verify blah (0008-SYSA-001, 0008-SYSA-002)`
4. If a feature test intentionally covers something that isn't explicitly an acceptance criteria
Expand Down
28 changes: 25 additions & 3 deletions bin/approbation.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,27 @@ const { checkFilenames } = require('../src/check-filenames')
const { nextFilename } = require('../src/next-filename')
const { nextCode } = require('../src/next-code')
const { checkCodes } = require('../src/check-codes')
const { checkFeatures } = require('../src/check-features')
const { checkReferences } = require('../src/check-references')
const pc = require('picocolors')
const { outputBranches } = require('../src/lib/get-project-branches')
const argv = require('minimist')(process.argv.slice(2))
const command = argv._[0]
const IS_DEBUG = process.env.debug !== undefined

function debugOutput (text) {
function debugOutput(text) {
if (IS_DEBUG) {
console.log(pc.purple(text))
}
}

function warn (lines) {
function warn(lines) {
console.warn('')
lines.map(l => console.warn(pc.yellow(`! ${l}`)))
console.warn('')
}

function showArg (arg, description) {
function showArg(arg, description) {
if (description) {
console.log(`${pc.bold(arg)}: ${description}`)
} else {
Expand Down Expand Up @@ -94,6 +95,27 @@ if (command === 'check-filenames') {

res = checkCodes(paths, ignoreGlob, isVerbose)
process.exit(res.exitCode)
} else if (command === 'check-features') {
let specs, features
const isVerbose = argv.verbose === true
const ignoreGlob = argv.ignore

if (!argv.features) {
warn(['No --features argument provided)'])
process.exit(1);
} else {
features = argv.features
}

if (!argv.specs) {
warn(['No --specs argument provided'])
process.exit(1);
} else {
specs = argv.specs
}

res = checkFeatures(specs, features, ignoreGlob, isVerbose)
process.exit(res.exitCode)
} else if (command === 'check-references') {
debugOutput('check-references')
const specsGlob = argv.specs
Expand Down
86 changes: 86 additions & 0 deletions src/check-features.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* Performs 2 checks on a given feature json file:
* 1. Checks that there are no duplicate AC codes in a single feature
* 2. Checks that all ACs exist in the current branch
*/
const fs = require('fs')
const glob = require('fast-glob')
const path = require('path')
const { validSpecificationPrefix, ignoreFiles } = require('./lib')
const { minimumAcceptableACsPerSpec } = require('./config')
const pc = require('picocolors')
const { gatherSpecs, findDuplicateAcs } = require('./check-references')
const { setFeatures } = require('./lib/feature')

function checkFeatures(paths, featuresPath, ignoreGlob, isVerbose = false) {
verbose = isVerbose
const ignoreList = ignoreGlob ? glob.sync(ignoreGlob, {}) : []
const fileList = ignoreFiles(glob.sync(paths, {}), ignoreList)
let exitCode = 0
let res, specs, features
let allCriteriaInFeatures = []

if (fileList.length > 0) {
specs = gatherSpecs(fileList)
features = setFeatures(JSON.parse(fs.readFileSync(featuresPath)))

let allCriteriaInSpecs = []
specs.forEach(spec => {
allCriteriaInSpecs = allCriteriaInSpecs.concat(spec.criteria)
})

Object.keys(features).filter(k => k !== 'Unknown').forEach(key => {
const c = features[key]
allCriteriaInFeatures = allCriteriaInFeatures.concat(c.acs)
const mysteryFeatureAcs = []
c.acs.forEach(ac => {
if (allCriteriaInSpecs.indexOf(ac) === -1) {
mysteryFeatureAcs.push(ac)
}
})

const duplicateAcs = findDuplicateAcs(c.acs);
if (duplicateAcs.length > 0 || mysteryFeatureAcs.length > 0) {
console.group(pc.bold(`Feature errors: ${key}`))

console.group();
if (duplicateAcs.length > 0) {
console.log(pc.red(pc.bold(`Duplicate ACs for ${key}(${c.milestone})`)) + ` ${duplicateAcs.join(', ')}`)
}

if (mysteryFeatureAcs.length > 0) {
console.log(pc.red(pc.bold(`Mystery ACs for ${key}(${c.milestone})`)) + ` ${JSON.stringify(mysteryFeatureAcs)} `)
}

console.groupEnd()
console.groupEnd()
console.log();
exitCode = 1;
}
})

if (isVerbose) {
const criteriaNotInFeatures = allCriteriaInSpecs.filter(ac => allCriteriaInFeatures.indexOf(ac) === -1)

if (criteriaNotInFeatures.length > 0) {
console.group(pc.yellow(pc.bold(`Criteria in specs, but not in features.json: `)) + `${criteriaNotInFeatures.length} out of ${allCriteriaInSpecs.length}`)
console.log(pc.yellow(criteriaNotInFeatures.join(', ')))
console.groupEnd()
console.log();
exitCode = 1;
}
}
} else {
console.error(pc.red(`glob matched no files (${paths})`))
exitCode = 1
}

return {
exitCode,
res
}
}

module.exports = {
checkFeatures
}
Loading

0 comments on commit 23bcc62

Please sign in to comment.