Skip to content

Commit

Permalink
add validate command (#31)
Browse files Browse the repository at this point in the history
* adding a validate command to help validate CODEOWNERS file

* fixing vuln in dev dependency
  • Loading branch information
jjmschofield authored Aug 3, 2020
1 parent 355446d commit 46f5cf7
Show file tree
Hide file tree
Showing 17 changed files with 277 additions and 29 deletions.
39 changes: 31 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
A CLI tool for working with GitHub CODEOWNERS.

Things it does:
* Output who owns each file in your repo (ignoring files listed in `.gitignore`)
* Output who owns a single file
* Output ownership information at a specific git commit
* Output ownership information of staged files
* Outputs lots of lovely stats
* Outputs handy formats for integrations (CSV and JSONL)
* Validates that the provided owners are in the correct format for github
* Calculate ownership stats
* Find out who owns each and every file (ignoring files listed in `.gitignore`)
* Find out who owns a single file
* Find out who owns your staged files
* Outputs in a bunch of script friendly handy formats for integrations (CSV and JSONL)
* Validates that your CODEOWNERS file is valid

## Installation
Install via npm globally then run
Expand Down Expand Up @@ -82,7 +81,7 @@ Usage: github-codeowners audit [options]
list the owners for all files

Options:
-d, --dir <dirPath> path to VCS directory (default: "/Users/jjmschofield/projects/github/github-codeowners")
-d, --dir <dirPath> path to VCS directory (default: "<current working directory>")
-c, --codeowners <filePath> path to codeowners file (default: "<dir>/.github/CODEOWNERS")
-o, --output <outputFormat> how to output format eg: simple, jsonl, csv (default: "simple")
-u, --unloved unowned files only (default: false)
Expand Down Expand Up @@ -152,6 +151,30 @@ Options:
-h, --help output usage information
```

### Validate
Validates your CODEOWNERS file to find common mistakes, will throw on errors (such as malformed owners).
```shell script
$ cd <your awesome project>
$ github-codeowners validate
Found duplicate rules [ 'some/duplicate/rule @octocat' ]
Found rules which did not match any files [ 'some/non-existent/path @octocat' ]
...
```

Full usage information:
```shell script
$ github-codeowners validate --help
Usage: github-codeowners validate [options]

Validates a CODOWNER file and files in dir

Options:
-d, --dir <dirPath> path to VCS directory (default: "<current working directory>")
-c, --codeowners <filePath> path to codeowners file (default: "<dir>/.github/CODEOWNERS")
-r, --root <rootPath> the root path to filter files by (default: "")
-h, --help output usage information
```

## Output Formats
Check `github-codeowners <command> --help` for support for a given command, however generally the following outputs are supported:
* `simple` - tab delimited - terminal friendly output
Expand Down
40 changes: 25 additions & 15 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"jest-junit": "^11.0.1",
"ts-jest": "^26.1.3",
"ts-node": "^8.6.2",
"tslint": "^6.0.0",
"tslint": "^6.1.3",
"tslint-config-airbnb-base": "^0.3.0",
"typescript": "^3.8.2",
"uuid": "^8.2.0"
Expand Down
24 changes: 24 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { git } from './commands/git';
import { log } from './lib/logger';

import { OUTPUT_FORMAT } from './lib/writers/types';
import { validate } from './commands/validate';

commander.command('audit')
.description('list the owners for all files')
Expand All @@ -35,6 +36,29 @@ commander.command('audit')
}
});

commander.command('validate')
.description('Validates a CODOWNER file and files in dir')
.option('-d, --dir <dirPath>', 'path to VCS directory', process.cwd())
.option('-c, --codeowners <filePath>', 'path to codeowners file (default: "<dir>/.github/CODEOWNERS")')
.option('-r, --root <rootPath>', 'the root path to filter files by', '')
.action(async (options) => {
try {
if (!options.codeowners) {
options.codeowners = path.resolve(options.dir, '.github/CODEOWNERS');
}

if (options.root) {
options.dir = path.resolve(options.dir, options.root);
}

await validate(options);
} catch (error) {
log.error('failed to run validate command', error);
process.exit(1);
}
});


commander.command('who <file>')
.description('lists owners of a specific file')
.option('-d, --dir <dirPath>', 'path to VCS directory', process.cwd())
Expand Down
6 changes: 6 additions & 0 deletions src/commands/__fixtures__/validate/files.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"files" : [
{ "path": "valid.js" },
{ "path": "duplicate-rule.js" }
]
}
16 changes: 16 additions & 0 deletions src/commands/__fixtures__/validate/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import path from 'path';

import { FixturePaths } from '../types';

const paths: FixturePaths = {
files: path.resolve(__dirname, 'files.json'),
codeowners: path.resolve(__dirname, 'owners'),
gitignores: [],
};

export const invalidOwnerFixtures: FixturePaths = {
...paths,
codeowners: path.resolve(__dirname, 'owners-invalid-format'),
};

export default paths;
4 changes: 4 additions & 0 deletions src/commands/__fixtures__/validate/owners
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
valid.js @some-owner
duplicate-rule.js @some-owner
file-does-not-exist.md @some-owner
duplicate-rule.js @some-owner
1 change: 1 addition & 0 deletions src/commands/__fixtures__/validate/owners-invalid-format
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
file-exists-duplicate-rule.md @valid-owner not-a-valid-owner
9 changes: 9 additions & 0 deletions src/commands/__snapshots__/validate.test.int.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`validate when all owners are valid output the result of all validation checks: stderr 1`] = `
"Found duplicate rules [ 'duplicate-rule.js @some-owner' ]
Found rules which did not match any files [ 'file-does-not-exist.md @some-owner' ]
"
`;

exports[`validate when all owners are valid output the result of all validation checks: stdout 1`] = `""`;
52 changes: 52 additions & 0 deletions src/commands/validate.test.int.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { v4 as uuidv4 } from 'uuid';
import fixtures, { invalidOwnerFixtures } from './__fixtures__/validate';
import { generateProject } from './__fixtures__/project-builder.test.helper';

import util from 'util';

const exec = util.promisify(require('child_process').exec);

describe('validate', () => {
describe('when all owners are valid', () => {
const testId = uuidv4();

let testDir = 'not set';

beforeAll(async () => {
testDir = await generateProject(testId, fixtures);
// tslint:disable-next-line:no-console
console.log(`test scratch dir: ${testDir}`);
});

const runCli = async (args: string) => {
return exec(`node ../../../dist/cli.js ${args}`, { cwd: testDir });
};

it('output the result of all validation checks', async () => {
const { stdout, stderr } = await runCli('validate');
expect(stdout).toMatchSnapshot('stdout');
expect(stderr).toMatchSnapshot('stderr');
});
});


describe('when owners are invalid', () => {
const testId = uuidv4();

let testDir = 'not set';

beforeAll(async () => {
testDir = await generateProject(testId, invalidOwnerFixtures);
// tslint:disable-next-line:no-console
console.log(`test scratch dir: ${testDir}`);
});

const runCli = async (args: string) => {
return exec(`node ../../../dist/cli.js ${args}`, { cwd: testDir });
};

it('should throw on invalid users', async () => {
await expect(() => runCli('validate')).rejects.toThrow();
});
});
});
21 changes: 21 additions & 0 deletions src/commands/validate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// import { writeOwnedFile, writeStats } from '../lib/writers';
import { validate as assertValidRules } from '../lib/ownership';
import { log } from '../lib/logger';

interface ValidateOptions {
codeowners: string;
dir: string;
root: string;
}

export const validate = async (options: ValidateOptions) => {
const results = await assertValidRules(options); // will throw on errors such as badly formatted rules

if(results.duplicated.size > 0){
log.warn('Found duplicate rules', Array.from(results.duplicated.values()));
}

if(results.unmatched.size > 0){
log.warn('Found rules which did not match any files', Array.from(results.unmatched.values()));
}
};
4 changes: 2 additions & 2 deletions src/lib/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ class Logger {
console.error(msg, error);
}

public warn(msg: string, error?: Error): void {
public warn(msg: string, obj?: Object): void {
// tslint:disable-next-line:no-console
console.warn(msg, error);
console.warn(msg, obj);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/ownership/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { OwnedFile } from './lib/OwnedFile';
export { OwnershipEngine } from './lib/OwnershipEngine';
export { getFileOwnership } from './file';
export { validate } from './validate';
export { Matcher, FileOwnershipMatcher } from './types';
19 changes: 19 additions & 0 deletions src/lib/ownership/lib/OwnershipEngine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ describe('OwnershipEngine', () => {
describe('calcFileOwnership', () => {
const createFileOwnershipMatcher = (path: string, owners: string[]): FileOwnershipMatcher => {
return {
rule: `${path} ${owners.join(' ')}`,
path,
owners,
match: (testPath: string) => {
return testPath === path;
},
matched: 0,
};
};

Expand All @@ -40,6 +42,23 @@ describe('OwnershipEngine', () => {
expect(result).toEqual(expectedOwners);
});

it('should count the number of times a rule is matched to a path', () => {
// Arrange
const owners = ['@owner-1', '@owner-2'];
const path = 'my/awesome/file.ts';
const matcher = createFileOwnershipMatcher(path, owners);

expect(matcher.matched).toEqual(0);

const underTest = new OwnershipEngine([matcher]);

// Act
underTest.calcFileOwnership(path);

// Assert
expect(underTest.getRules()[0].matched).toEqual(1);
});

it('should should take precedence from the last matching rule', () => {
// Arrange
const expectedOwner = '@owner-2';
Expand Down
Loading

0 comments on commit 46f5cf7

Please sign in to comment.