Skip to content

Commit 67f22ae

Browse files
FSVetazskye2k2
andauthored
Compare Final Config for Testing (#10)
Print out final eslint config with the rules sorted alphabetically. Compare the config to the previous reason in a new test. Clean up output files to strip out environment-specific data via node script, as sed was causing too many issues. Update documentation. --------- Co-authored-by: Clif Bergmann <[email protected]>
1 parent 192e242 commit 67f22ae

11 files changed

+1465
-105
lines changed

.gitignore

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@
1111
/reports/**
1212
analysis.json
1313
npm-debug.log
14-
/demo/test/snapshots/*results.txt
14+
/demo/test/snapshots/local*

README.md

+17-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,22 @@
22

33
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).
44

5-
Why? Because we believe in linting, and we have become converted to the additional rules enforced by the following plugins:
5+
## Unit tests
6+
7+
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.
8+
9+
**Process:**
10+
11+
1. Make dependency/configuration updates.
12+
1. Run `npm test`.
13+
- The tests should likely fail. Verify your expectations against the current [snapshot](/demo/test/snapshots/linting-config.test.js.md) file.
14+
1. After you have your results how you want them, run `npm run test:update`.
15+
- The tests should now pass.
16+
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.
17+
18+
> TODO: Update the documentation below to be current, and not include things like Code Climate
19+
20+
Why extra rules? Because we believe in linting, and we have become converted to the additional rules enforced by the following plugins:
621

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

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

127-
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.
142+
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.
128143

129144
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.
130145

demo/example.js

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
// Hack: Note that these work, regardless of case
1313
// Here be Dragons
1414

15+
/**
16+
* 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.
17+
*/
18+
1519
function functionWithoutJSDocWarningsBecauseTheSectionWasCompletelyExcluded () {
1620
console.log('ASHLDKFJHASKFJSDHFKJSDHFKLSDJHFLJKSDHFLKSDJFHKSDLJFHLSDKJF');
1721
return true;

demo/test/lint-output.js

-15
This file was deleted.

demo/test/linting-config.test.js

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// 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.
2+
3+
import test from 'ava';
4+
const fileManager = require('file-manager-js');
5+
6+
function processFile (t, filename) {
7+
// Run previously via npm test, save off results, and read output
8+
return fileManager.readFile(`./demo/test/snapshots/${filename}`)
9+
.then((content) => {
10+
const output = content.toString(); // content is instance of Buffer, so it needs to be parsed
11+
return t.snapshot(output);
12+
})
13+
.catch((err) => {
14+
console.log(err); // eslint-disable-line no-console -- Tests use the console.
15+
return t.fail();
16+
});
17+
}
18+
19+
test('Should apply our custom linting rules consistently', async t => {
20+
return processFile(t, 'local-linting-output.txt');
21+
});
22+
23+
test('Should apply a consistent overall eslint configuration', async t => {
24+
return processFile(t, 'local-linting-final-config.json'); // If this fails, go cry to mommy
25+
});

demo/test/snapshots/format-config.js

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Process the exported linting results and final configuration files:
2+
// Sort final configuration rules alphabetically to compare changes easier.
3+
// Remove any developer-specific directory paths for both files.
4+
5+
/* eslint no-console: "off" -- node scripts use the console, so disable for the whole file */
6+
7+
const finalConfig = require('./local-linting-final-config.json');
8+
9+
const FS = require('fs');
10+
11+
const formattedRules = Object.fromEntries(
12+
Object.entries(finalConfig?.rules ?? {}).sort(([ruleNameA], [ruleNameB]) => {
13+
if (ruleNameA > ruleNameB) return 1;
14+
if (ruleNameB > ruleNameA) return -1;
15+
return 0;
16+
})
17+
);
18+
19+
FS.writeFile(
20+
'./demo/test/snapshots/local-linting-final-config.json',
21+
JSON.stringify(
22+
{ ...finalConfig, rules: formattedRules, parser: finalConfig.parser?.split('eslint-config-tree')[1] },
23+
null,
24+
2
25+
),
26+
(err) => {
27+
if (err) console.log('There was an error writing to local-linting-final-config.json file:', err);
28+
}
29+
);
30+
31+
FS.readFile('./demo/test/snapshots/local-linting-output.txt', 'utf8', (err, eslintOutput) => {
32+
if (err) {
33+
console.log('There was an error reading local-linting-output.txt', err);
34+
} else {
35+
FS.writeFile(
36+
'./demo/test/snapshots/local-linting-output.txt',
37+
eslintOutput.replace(/.*demo\//g, ''),
38+
(err2) => {
39+
if (err2) console.log('There was an error writing to local-linting-output.txt file:', err);
40+
}
41+
);
42+
}
43+
});

demo/test/snapshots/lint-output.js.md

-81
This file was deleted.
-2.06 KB
Binary file not shown.

0 commit comments

Comments
 (0)