Skip to content

Compare Final Config for Testing #10

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

Merged
merged 10 commits into from
Feb 24, 2023
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
/reports/**
analysis.json
npm-debug.log
/demo/test/snapshots/*results.txt
/demo/test/snapshots/local*
19 changes: 17 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,22 @@

This is a shared configuration for all Tree repositories. Contains overrides and enhancements on top of the base configuration located at [https://github.com/fs-webdev/eslint-config-frontier](https://github.com/fs-webdev/eslint-config-frontier).

Why? Because we believe in linting, and we have become converted to the additional rules enforced by the following plugins:
## Unit tests

This central configuration is a potential breaking point for _all_ of our code if we suddenly break our rules, so we have tests in place that verify that our configuration remains consistent between upgrades (primarily that we _know_ what changed), and that the extended cases that we care about are still caught. We do this by utilizing ava's snapshot ability against exported (and slightly modified) linting output and linting configuration. These files are not committed, as they are re-created on each test run, but the resulting snapshot and summary markdown file are part of version control, to make it easier to see changes.

**Process:**

1. Make dependency/configuration updates.
1. Run `npm test`.
- The tests should likely fail. Verify your expectations against the current [snapshot](/demo/test/snapshots/linting-config.test.js.md) file.
1. After you have your results how you want them, run `npm run test:update`.
- The tests should now pass.
1. If you want see how your changes would impact a codebase, you can either `npm link` or copy+paste the contents of `local-linting-final-config.json` temporarily into the target `.eslintrc` file.

> TODO: Update the documentation below to be current, and not include things like Code Climate

Why extra rules? Because we believe in linting, and we have become converted to the additional rules enforced by the following plugins:

- [eslint-plugin-bestpractices](https://github.com/skye2k2/eslint-plugin-bestpractices)
- [eslint-plugin-deprecate](https://github.com/AlexMost/eslint-plugin-deprecate)
Expand Down Expand Up @@ -124,7 +139,7 @@ Or disable BOTH the desired rule and the no-eslint-disable rule:

### How to deal with `Definition for rule '{RULE}' was not found.` errors:

This is a known state when submitting a new file to Code Climate for the first time, since they do not support all of the linting extensions we wish to use. If you are seeing these warnings when linting locally, you may have `eslint` installed globally, but not the additional dependency. We do not recommend running `eslint` globally for this reason (see: https://github.com/eslint/eslint/issues/6732). All Tree repositories should include all dependencies required to be able to run `eslint` locally in their respective directories.
This is a known state when submitting a new file to Code Climate for the first time, since they do not support all of the linting extensions we wish to use. If you are seeing these warnings when linting locally, you may have `eslint` installed globally, but not the additional dependency. We do not recommend running `eslint` globally for this reason (see: https://github.com/eslint/eslint/issues/6732). All Tree repositories should include all dependencies required to be able to run `eslint` locally in their respective directories.

If you have recently updated dependencies and see this error locally, then there is a possibility that your editor's linting integration is out-of-sync that can be resolved by restarting your editor.

Expand Down
4 changes: 4 additions & 0 deletions demo/example.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
// Hack: Note that these work, regardless of case
// Here be Dragons

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh that is interesting

* As long as you separate the comment block from the declaration, JSDOC rules will not be applied. But the comment will still show through many IDE's definition hot-linking.
*/

function functionWithoutJSDocWarningsBecauseTheSectionWasCompletelyExcluded () {
console.log('ASHLDKFJHASKFJSDHFKJSDHFKLSDJHFLJKSDHFLKSDJFHKSDLJFHLSDKJF');
return true;
Expand Down
15 changes: 0 additions & 15 deletions demo/test/lint-output.js

This file was deleted.

25 changes: 25 additions & 0 deletions demo/test/linting-config.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// NOTE: This test file runs against untracked files in an attempt to be an early warning system against making changes that would lose configuration that we care about. See the README for more information.

import test from 'ava';
const fileManager = require('file-manager-js');

function processFile (t, filename) {
// Run previously via npm test, save off results, and read output
return fileManager.readFile(`./demo/test/snapshots/${filename}`)
.then((content) => {
const output = content.toString(); // content is instance of Buffer, so it needs to be parsed
return t.snapshot(output);
})
.catch((err) => {
console.log(err); // eslint-disable-line no-console -- Tests use the console.
return t.fail();
});
}

test('Should apply our custom linting rules consistently', async t => {
return processFile(t, 'local-linting-output.txt');
});

test('Should apply a consistent overall eslint configuration', async t => {
return processFile(t, 'local-linting-final-config.json'); // If this fails, go cry to mommy
});
43 changes: 43 additions & 0 deletions demo/test/snapshots/format-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Process the exported linting results and final configuration files:
// Sort final configuration rules alphabetically to compare changes easier.
// Remove any developer-specific directory paths for both files.

/* eslint no-console: "off" -- node scripts use the console, so disable for the whole file */

const finalConfig = require('./local-linting-final-config.json');

const FS = require('fs');

const formattedRules = Object.fromEntries(
Object.entries(finalConfig?.rules ?? {}).sort(([ruleNameA], [ruleNameB]) => {
if (ruleNameA > ruleNameB) return 1;
if (ruleNameB > ruleNameA) return -1;
return 0;
})
);

FS.writeFile(
'./demo/test/snapshots/local-linting-final-config.json',
JSON.stringify(
{ ...finalConfig, rules: formattedRules, parser: finalConfig.parser?.split('eslint-config-tree')[1] },
null,
2
),
(err) => {
if (err) console.log('There was an error writing to local-linting-final-config.json file:', err);
}
);

FS.readFile('./demo/test/snapshots/local-linting-output.txt', 'utf8', (err, eslintOutput) => {
if (err) {
console.log('There was an error reading local-linting-output.txt', err);
} else {
FS.writeFile(
'./demo/test/snapshots/local-linting-output.txt',
eslintOutput.replace(/.*demo\//g, ''),
(err2) => {
if (err2) console.log('There was an error writing to local-linting-output.txt file:', err);
}
);
}
});
81 changes: 0 additions & 81 deletions demo/test/snapshots/lint-output.js.md

This file was deleted.

Binary file removed demo/test/snapshots/lint-output.js.snap
Binary file not shown.
Loading