Skip to content

Commit

Permalink
fix(): add docs for new checks; add tests; add 'negative tests' marke…
Browse files Browse the repository at this point in the history
…r to tests
  • Loading branch information
mikaelvesavuori committed Jul 8, 2023
1 parent 4688474 commit 452b1b5
Show file tree
Hide file tree
Showing 25 changed files with 225 additions and 24 deletions.
33 changes: 26 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ The format is:
| Key | Required | Default | Example | Description |
| ----------------- | -------- | --------------- | ----------------------------- | ----------------------------------------------------------------------- |
| `basePath` | No | `.` | `./project_dir/` | Sets the base path for any file lookups. |
| `ignorePaths` | No | `[]` | `["/tests/", "/src/problematic-file.ts"]` | Paths that will be ignored for some checks. Does not support globbing. |
| `checks` | Yes | - | `["checkForPresenceLicense"]` | A list of checks to run, either using string or object form. |
| `defaultSeverity` | No | `error` | `warn` | Sets the default severity level for any issues. |
| `path` | No | Multiple values | `api/schema.yml` | Sets the exact path to a resource. Only used optionally by some checks. |
Expand All @@ -74,6 +75,16 @@ If you for some reason keep your project files "lower" than in the root where yo

_It's recommended you do not change this unless you know what you are doing._

#### Ignore paths

This provides the possibility to ignore certain paths.

These paths will be respected and discarded by the following checks:

- `checkForConsoleUsage`
- `checkForPresenceTests`
- `checkForThrowingPlainErrors`

#### Checks

Checks can be provided in string form or object form:
Expand Down Expand Up @@ -158,6 +169,8 @@ Checks if `console` is used, e.g. `console.log()` and similar. This is useful wh

Note that this uses a naive solution so even just a mention of console.log() (or similar) will fail this check.

**Will respect `ignorePaths` options.**

**Default**: `src`

### `checkForDefinedRelations`
Expand Down Expand Up @@ -250,18 +263,24 @@ Checks if there are any tests in the provided location. This will match anything

**Default**: `.github/ISSUE_TEMPLATE/pull_request.md`

**Will respect `ignorePaths` options.**

### `checkForThrowingPlainErrors`

Checks if any file uses `throw Error` or `throw new Error`. This is meant to push toward the use of actual loggers, rather than plain console output.

**Will respect `ignorePaths` options.**

---

## Ideas for improvements

Checks:

- Do:
- Check for throwing native errors
- Ensure methods/functions are below a certain threshold in terms of lines of code
- Documentation coverage
- Ensure imports follow acceptable conventions
- No cyclic methods/dependencies
- Do later:
- Ensure methods/functions are below a certain threshold in terms of lines of code
- Documentation coverage
- Ensure imports follow certain conventions
- No cyclic methods/dependencies
- Maybe:
- Service metadata: Do you link to observability resources (logs/metrics/traces/dashboards etc.)?
- Support for external config
3 changes: 2 additions & 1 deletion src/checks/checkForConsoleUsage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export function checkForConsoleUsage(
if (!customPath) logDefaultPathMessage(name, path);

const files = getAllFiles(`${basePath}/${path}`, []);
const filteredFiles = ignorePaths ? filterFiles(files, ignorePaths) : files;
const filteredFiles =
ignorePaths && ignorePaths.length > 0 ? filterFiles(files, ignorePaths) : files;

const regex = /console.(.*)/gi;
const includesConsole = filteredFiles.map((test: string) => regex.test(readFile(test)));
Expand Down
3 changes: 2 additions & 1 deletion src/checks/checkForPresenceTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export function checkForPresenceTests(
if (!customPath) logDefaultPathMessage(name, path);

const files = getAllFiles(`${basePath}/${path}`, []);
const filteredFiles = ignorePaths ? filterFiles(files, ignorePaths) : files;
const filteredFiles =
ignorePaths && ignorePaths.length > 0 ? filterFiles(files, ignorePaths) : files;

const tests = filteredFiles.filter(
(file: string) =>
Expand Down
3 changes: 2 additions & 1 deletion src/checks/checkForThrowingPlainErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export function checkForThrowingPlainErrors(
if (!customPath) logDefaultPathMessage(name, path);

const files = getAllFiles(`${basePath}/${path}`, []);
const filteredFiles = ignorePaths ? filterFiles(files, ignorePaths) : files;
const filteredFiles =
ignorePaths && ignorePaths.length > 0 ? filterFiles(files, ignorePaths) : files;

const regex = /(throw Error|throw new Error)(.*)/gi;
const includesError = filteredFiles.map((test: string) => regex.test(readFile(test)));
Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForConflictingLockfiles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when there are no conflicting lock files', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when there are conflicting lock files', (t) => {
const expected = 'warn';

Expand Down
33 changes: 22 additions & 11 deletions tests/checks/checkForConsoleUsage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import test from 'ava';

import { createNewStandardLint } from '../../src/domain/StandardLint';

test('It should pass when not finding any console usage when using base path', (t) => {
test('It should pass when not finding any console usage while using base path', (t) => {
const expected = 'pass';

const standardlint = createNewStandardLint({
Expand All @@ -14,7 +14,7 @@ test('It should pass when not finding any console usage when using base path', (
t.deepEqual(result, expected);
});

test('It should pass when not finding any console usage when using check-only path', (t) => {
test('It should pass when not finding any console usage while using check-only path', (t) => {
const expected = 'pass';

const standardlint = createNewStandardLint({
Expand All @@ -26,13 +26,12 @@ test('It should pass when not finding any console usage when using check-only pa
t.deepEqual(result, expected);
});

test('It should pass and accept ignore paths', (t) => {
test('It should pass when not finding any console usage while using ignore paths', (t) => {
const expected = 'pass';

const standardlint = createNewStandardLint({
basePath: 'testdata',
ignorePaths: ['/src/'],
checks: [{ name: 'checkForConsoleUsage' }]
checks: ['checkForConsoleUsage']
});
const result = standardlint.check().results?.[0]?.status;

Expand All @@ -43,36 +42,48 @@ test('It should pass and accept ignore paths', (t) => {
* NEGATIVE TESTS
*/

test('It should fail when finding console.log()', (t) => {
test('It should warn when finding console.log()', (t) => {
const expected = 'warn';

const standardlint = createNewStandardLint({
basePath: 'testdata',
checks: [{ name: 'checkForConsoleUsage', severity: 'warn' }]
});
const result = standardlint.check().results?.[0]?.status;

t.deepEqual(result, expected);
});

test('It should error when finding console.log()', (t) => {
const expected = 'fail';

const standardlint = createNewStandardLint({
basePath: 'testdata',
checks: ['checkForConsoleUsage']
checks: [{ name: 'checkForConsoleUsage', severity: 'error' }]
});
const result = standardlint.check().results?.[0]?.status;

t.deepEqual(result, expected);
});

test('It should fail when finding console.warn()', (t) => {
test('It should error when finding console.warn()', (t) => {
const expected = 'fail';

const standardlint = createNewStandardLint({
basePath: 'testdata',
checks: ['checkForConsoleUsage']
checks: [{ name: 'checkForConsoleUsage', severity: 'error' }]
});
const result = standardlint.check().results?.[0]?.status;

t.deepEqual(result, expected);
});

test('It should fail when finding console.error()', (t) => {
test('It should error when finding console.error()', (t) => {
const expected = 'fail';

const standardlint = createNewStandardLint({
basePath: 'testdata',
checks: ['checkForConsoleUsage']
checks: [{ name: 'checkForConsoleUsage', severity: 'error' }]
});
const result = standardlint.check().results?.[0]?.status;

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForDefinedRelations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding at least one defined relation', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when finding no defined relations', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForDefinedServiceLevelObjectives.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding at least one defined Service Level Objective',
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when finding no defined Service Level Objectives', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForDefinedTags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding at least one defined tag', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when finding no defined tags', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceApiSchema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding an API schema', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing an API schema', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceChangelog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a CHANGELOG file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a CHANGELOG file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceCiConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a CI configuration file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a CI configuration file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceCodeowners.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a CODEOWNERS file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a CODEOWNERS file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceContributing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a CONTRIBUTING file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a CONTRIBUTING file', (t) => {
const expected = 'warn';

Expand Down
6 changes: 5 additions & 1 deletion tests/checks/checkForPresenceDiagramsFolder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a folder with solution diagrams', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a folder with solution diagrams', (t) => {
const expected = 'warn';

Expand All @@ -38,7 +42,7 @@ test('It should error when missing a folder with solution diagrams', (t) => {
t.deepEqual(result, expected);
});

test('It should fail when using a non-existent directory', (t) => {
test('It should error when using a non-existent directory', (t) => {
const expected = 'fail';

const standardlint = createNewStandardLint({
Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceIacConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding an Infrastructure-as-Code configuration', (t)
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing an Infrastructure-as-Code configuration', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceLicense.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a LICENSE file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a LICENSE file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceReadme.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a README file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a README file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceSecurity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a SECURITY file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a SECURITY file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceServiceMetadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a service metadata file', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a service metadata file', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceTemplateIssues.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a GitHub issue template', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a GitHub issue template', (t) => {
const expected = 'warn';

Expand Down
4 changes: 4 additions & 0 deletions tests/checks/checkForPresenceTemplatePullRequests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ test('It should pass when finding a GitHub Pull Request template', (t) => {
t.deepEqual(result, expected);
});

/**
* NEGATIVE TESTS
*/

test('It should warn when missing a GitHub Pull Request template', (t) => {
const expected = 'warn';

Expand Down
Loading

0 comments on commit 452b1b5

Please sign in to comment.