From b2e7cd6ada710715fca67c3be58efa3dacd25618 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Mon, 24 Apr 2017 20:56:46 +0200 Subject: [PATCH 1/8] extract the reporter out of integrity checker --- src/cli/commands/check.js | 11 ++++- src/cli/commands/install.js | 2 +- src/integrity-checker.js | 94 ++++++++++++++++++------------------- 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/src/cli/commands/check.js b/src/cli/commands/check.js index c09593c750..d590d939e8 100644 --- a/src/cli/commands/check.js +++ b/src/cli/commands/check.js @@ -147,7 +147,15 @@ async function integrityHashCheck( reporter.error(reporter.lang(msg, ...vars)); errCount++; } - const integrityChecker = new InstallationIntegrityChecker(config, reporter); + const reasons = { + 'EXPECTED_MISSING': 'integrityFailedExpectedMissing', + 'FILES_MISSING': 'integrityFailedFilesMissing', + 'LOCKFILE_DONT_MATCH': 'integrityLockfilesDontMatch', + 'FLAGS_DONT_MATCH': 'integrityFlagsDontMatch', + 'PATTERNS_DONT_MATCH': 'integrityPatternsDontMatch', + 'LINKED_MODULES_DONT_MATCH': 'integrityCheckLinkedModulesDontMatch', + }; + const integrityChecker = new InstallationIntegrityChecker(config); const lockfile = await Lockfile.fromDirectory(config.cwd); const install = new Install(flags, config, reporter, lockfile); @@ -163,6 +171,7 @@ async function integrityHashCheck( reportError('noIntegrityFile'); } if (!match.integrityMatches) { + reportError(reasons[match.whyIntegrityMatchesFailed]); reportError('integrityCheckFailed'); } diff --git a/src/cli/commands/install.js b/src/cli/commands/install.js index 32228cfd26..917ac9dd79 100644 --- a/src/cli/commands/install.js +++ b/src/cli/commands/install.js @@ -172,7 +172,7 @@ export class Install { this.resolver = new PackageResolver(config, lockfile); this.fetcher = new PackageFetcher(config, this.resolver); - this.integrityChecker = new InstallationIntegrityChecker(config, this.reporter); + this.integrityChecker = new InstallationIntegrityChecker(config); this.compatibility = new PackageCompatibility(config, this.resolver, this.flags.ignoreEngines); this.linker = new PackageLinker(config, this.resolver); this.scripts = new PackageInstallScripts(config, this.resolver, this.flags.force); diff --git a/src/integrity-checker.js b/src/integrity-checker.js index 848d645073..8e856d58ac 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -3,7 +3,6 @@ import type Config from './config.js'; import type {LockManifest} from './lockfile/wrapper.js'; import type {RegistryNames} from './registries/index.js'; -import type {Reporter} from './reporters/index.js'; import * as constants from './constants.js'; import {registryNames} from './registries/index.js'; import * as fs from './util/fs.js'; @@ -47,15 +46,11 @@ type IntegrityFlags = { export default class InstallationIntegrityChecker { constructor( config: Config, - reporter: Reporter, ) { this.config = config; - this.reporter = reporter; } config: Config; - reporter: Reporter; - /** * Get the location of an existing integrity hash. If none exists then return the location where we should @@ -167,32 +162,61 @@ export default class InstallationIntegrityChecker { return result; } - _compareIntegrityFiles(actual: IntegrityFile, expected: IntegrityFile): boolean { + async _getIntegrityFile(locationPath: string): Promise { + const expectedRaw = await fs.readFile(locationPath); + try { + return JSON.parse(expectedRaw); + } catch (e) { + // ignore JSON parsing for legacy text integrity files compatibility + } + return null; + } + + async _compareIntegrityFiles( + actual: IntegrityFile, + expected: ?IntegrityFile, + checkFiles: boolean, + locationFolder: string): Promise { + if (!expected) { + return Promise.resolve('EXPECTED_MISSING'); + } if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) { - this.reporter.warn(this.reporter.lang('integrityCheckLinkedModulesDontMatch')); - return false; + return Promise.resolve('LINKED_MODULES_DONT_MATCH'); } if (!compareSortedArrays(actual.topLevelPatters, expected.topLevelPatters)) { - this.reporter.warn(this.reporter.lang('integrityPatternsDontMatch')); - return false; + return Promise.resolve('PATTERNS_DONT_MATCH'); } if (!compareSortedArrays(actual.flags, expected.flags)) { - this.reporter.warn(this.reporter.lang('integrityFlagsDontMatch')); - return false; + return Promise.resolve('FLAGS_DONT_MATCH'); } for (const key of Object.keys(actual.lockfileEntries)) { if (actual.lockfileEntries[key] !== expected.lockfileEntries[key]) { - this.reporter.warn(this.reporter.lang('integrityLockfilesDontMatch')); - return false; + return Promise.resolve('LOCKFILE_DONT_MATCH'); } } for (const key of Object.keys(expected.lockfileEntries)) { if (actual.lockfileEntries[key] !== expected.lockfileEntries[key]) { - this.reporter.warn(this.reporter.lang('integrityLockfilesDontMatch')); - return false; + return Promise.resolve('LOCKFILE_DONT_MATCH'); + } + } + if (checkFiles) { + if (expected.files.length === 0) { + // edge case handling - --check-fies is passed but .yarn-integrity does not contain any files + // check and fail if there are file in node_modules after all. + const actualFiles = await this._getFilesDeep(locationFolder); + if (actualFiles.length > 0) { + return Promise.resolve('FILES_MISSING'); + } + } else { + // TODO we may want to optimise this check by checking only for package.json files on very large trees + for (const file of expected.files) { + if (!await fs.exists(path.join(locationFolder, file))) { + return Promise.resolve('FILES_MISSING'); + } + } } } - return true; + return Promise.resolve('OK'); } async check( @@ -214,41 +238,13 @@ export default class InstallationIntegrityChecker { patterns, Object.assign({}, {checkFiles: false}, flags), // don't generate files when checking, we check the files below loc.locationFolder); - const expectedRaw = await fs.readFile(loc.locationPath); - let expected: ?IntegrityFile; - try { - expected = JSON.parse(expectedRaw); - } catch (e) { - // ignore JSON parsing for legacy text integrity files compatibility - } - let integrityMatches; - if (expected) { - integrityMatches = this._compareIntegrityFiles(actual, expected); - if (flags.checkFiles && expected.files.length === 0) { - // edge case handling - --check-fies is passed but .yarn-integrity does not contain any files - // check and fail if there are file in node_modules after all. - const actualFiles = await this._getFilesDeep(loc.locationFolder); - if (actualFiles.length > 0) { - this.reporter.warn(this.reporter.lang('integrityFailedFilesMissing')); - integrityMatches = false; - } - } else if (flags.checkFiles && expected.files.length > 0) { - // TODO we may want to optimise this check by checking only for package.json files on very large trees - for (const file of expected.files) { - if (!await fs.exists(path.join(loc.locationFolder, file))) { - this.reporter.warn(this.reporter.lang('integrityFailedFilesMissing')); - integrityMatches = false; - break; - } - } - } - } else { - integrityMatches = false; - } + const expected = await this._getIntegrityFile(loc.locationPath); + const integrityMatches = await this._compareIntegrityFiles(actual, expected, flags.checkFiles, loc.locationFolder); return { integrityFileMissing: false, - integrityMatches, + integrityMatches: integrityMatches === 'OK', + whyIntegrityMatchesFailed: integrityMatches, missingPatterns, }; } From 0815eb0c2d8fd96bffceb8a588667abe77562f2b Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 25 Apr 2017 18:22:32 +0200 Subject: [PATCH 2/8] use warning for messages from integrity-checker in check command --- src/cli/commands/check.js | 2 +- src/integrity-checker.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/commands/check.js b/src/cli/commands/check.js index d590d939e8..d75216f03e 100644 --- a/src/cli/commands/check.js +++ b/src/cli/commands/check.js @@ -171,7 +171,7 @@ async function integrityHashCheck( reportError('noIntegrityFile'); } if (!match.integrityMatches) { - reportError(reasons[match.whyIntegrityMatchesFailed]); + reporter.warn(reporter.lang(reasons[match.whyIntegrityMatchesFailed])); reportError('integrityCheckFailed'); } diff --git a/src/integrity-checker.js b/src/integrity-checker.js index 8e856d58ac..b0f3198e3b 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -291,5 +291,4 @@ export default class InstallationIntegrityChecker { await fs.unlink(loc.locationPath); } } - } From 543dc0da866dbc408125e8059cd89fa2555173dd Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 25 Apr 2017 19:16:58 +0200 Subject: [PATCH 3/8] add tests for integrity when integrity file is missing or is not a json --- __tests__/commands/_helpers.js | 4 ++-- __tests__/commands/check.js | 34 ++++++++++++++++++++++++++++++++-- src/cli/commands/check.js | 4 ++-- src/integrity-checker.js | 2 +- src/reporters/lang/en.js | 1 + 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/__tests__/commands/_helpers.js b/__tests__/commands/_helpers.js index fa1b21da73..68987908e1 100644 --- a/__tests__/commands/_helpers.js +++ b/__tests__/commands/_helpers.js @@ -65,7 +65,7 @@ export async function run( args: Array, flags: Object, name: string | { source: string, cwd: string }, - checkInstalled: ?(config: Config, reporter: R, install: T) => ?Promise, + checkInstalled: ?(config: Config, reporter: R, install: T, getStdout: () => string) => ?Promise, beforeInstall: ?(cwd: string) => ?Promise, ): Promise { let out = ''; @@ -131,7 +131,7 @@ export async function run( const install = await factory(args, flags, config, reporter, lockfile, () => out); if (checkInstalled) { - await checkInstalled(config, reporter, install); + await checkInstalled(config, reporter, install, () => out); } } catch (err) { throw new Error(`${err && err.stack} \nConsole output:\n ${out}`); diff --git a/__tests__/commands/check.js b/__tests__/commands/check.js index 657052993b..c03b66eaa7 100644 --- a/__tests__/commands/check.js +++ b/__tests__/commands/check.js @@ -21,7 +21,7 @@ const runCheck = buildRun.bind( }, ); -test.concurrent('--verify-tree should report wrong version ', async (): Promise => { +test.concurrent('--verify-tree should report wrong version', async (): Promise => { let thrown = false; try { await runCheck([], {verifyTree: true}, 'verify-tree-version-mismatch'); @@ -31,7 +31,7 @@ test.concurrent('--verify-tree should report wrong version ', async (): Promise< expect(thrown).toEqual(true); }); -test.concurrent('--verify-tree should report missing dependency ', async (): Promise => { +test.concurrent('--verify-tree should report missing dependency', async (): Promise => { let thrown = false; try { await runCheck([], {verifyTree: true}, 'verify-tree-not-found'); @@ -80,6 +80,36 @@ test.concurrent('--integrity should ignore comments and whitespaces in yarn.lock }); }); +test.concurrent('--integrity should fail if integrity file is missing', async (): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { + await fs.unlink(path.join(config.cwd, 'node_modules', '.yarn-integrity')); + + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + }); +}); + +test.concurrent('--integrity should fail if integrity file is not a json', async (): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { + await fs.writeFile(path.join(config.cwd, 'node_modules', '.yarn-integrity'), 'not a json'); + + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: integrity file is not a json'); + }); +}); + test.concurrent('--integrity should fail if yarn.lock has patterns changed', async (): Promise => { await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock')); diff --git a/src/cli/commands/check.js b/src/cli/commands/check.js index d75216f03e..45187d64f8 100644 --- a/src/cli/commands/check.js +++ b/src/cli/commands/check.js @@ -148,7 +148,7 @@ async function integrityHashCheck( errCount++; } const reasons = { - 'EXPECTED_MISSING': 'integrityFailedExpectedMissing', + 'EXPECTED_IS_NOT_A_JSON': 'integrityFailedExpectedIsNotAJSON', 'FILES_MISSING': 'integrityFailedFilesMissing', 'LOCKFILE_DONT_MATCH': 'integrityLockfilesDontMatch', 'FLAGS_DONT_MATCH': 'integrityFlagsDontMatch', @@ -170,7 +170,7 @@ async function integrityHashCheck( if (match.integrityFileMissing) { reportError('noIntegrityFile'); } - if (!match.integrityMatches) { + if (match.integrityMatches === false) { reporter.warn(reporter.lang(reasons[match.whyIntegrityMatchesFailed])); reportError('integrityCheckFailed'); } diff --git a/src/integrity-checker.js b/src/integrity-checker.js index b0f3198e3b..ed00b2f07f 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -178,7 +178,7 @@ export default class InstallationIntegrityChecker { checkFiles: boolean, locationFolder: string): Promise { if (!expected) { - return Promise.resolve('EXPECTED_MISSING'); + return Promise.resolve('EXPECTED_IS_NOT_A_JSON'); } if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) { return Promise.resolve('LINKED_MODULES_DONT_MATCH'); diff --git a/src/reporters/lang/en.js b/src/reporters/lang/en.js index b0fbe904de..f99f086544 100644 --- a/src/reporters/lang/en.js +++ b/src/reporters/lang/en.js @@ -254,6 +254,7 @@ const messages = { lockfileNotContainPattern: 'Lockfile does not contain pattern: $0', integrityCheckFailed: 'Integrity check failed', noIntegrityFile: 'Couldn\'t find an integrity file', + integrityFailedExpectedIsNotAJSON: 'Integrity check: integrity file is not a json', integrityCheckLinkedModulesDontMatch: 'Integrity check: Linked modules don\'t match', integrityPatternsDontMatch: 'Integrity check: Patterns don\'t match', integrityFlagsDontMatch: 'Integrity check: Flags don\'t match', From 0b06e7467a55c39224f15a8498a47a717dfd0be1 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 25 Apr 2017 19:29:29 +0200 Subject: [PATCH 4/8] add assert on pre-existent tests --- __tests__/commands/check.js | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/__tests__/commands/check.js b/__tests__/commands/check.js index c03b66eaa7..e202a7809e 100644 --- a/__tests__/commands/check.js +++ b/__tests__/commands/check.js @@ -127,7 +127,8 @@ test.concurrent('--integrity should fail if yarn.lock has patterns changed', asy }); test.concurrent('--integrity should fail if yarn.lock has new pattern', async (): Promise => { - await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock')); lockfile += `\nxtend@^4.0.0: version "4.0.1" @@ -141,11 +142,13 @@ test.concurrent('--integrity should fail if yarn.lock has new pattern', async () thrown = true; } expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Lock files don\'t match'); }); }); test.concurrent('--integrity should fail if yarn.lock has resolved changed', async (): Promise => { - await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock')); lockfile = lockfile.replace('https://registry.npmjs.org/left-pad/-/left-pad-1.1.1.tgz', 'https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.1.tgz'); @@ -158,13 +161,14 @@ test.concurrent('--integrity should fail if yarn.lock has resolved changed', asy thrown = true; } expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Lock files don\'t match'); }); }); test.concurrent('--integrity should fail if files are missing and --check-files is passed', async (): Promise => { await runInstall({checkFiles: true}, path.join('..', 'check', 'integrity-lock-check'), - async (config, reporter): Promise => { + async (config, reporter, install, getStdout): Promise => { await fs.unlink(path.join(config.cwd, 'node_modules', 'left-pad', 'index.js')); let thrown = false; @@ -174,22 +178,23 @@ async (): Promise => { thrown = true; } expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Files are missing'); }); }); -test.concurrent('--integrity should fail if --ignore-scripts is changed', - async (): Promise => { - await runInstall({ignoreScripts: true}, path.join('..', 'check', 'integrity-lock-check'), - async (config, reporter): Promise => { - let thrown = false; - try { - await checkCmd.run(config, reporter, {integrity: true, ignoreScripts: false}, []); - } catch (e) { - thrown = true; - } - expect(thrown).toEqual(true); - }); - }); +test.concurrent('--integrity should fail if --ignore-scripts is changed', async (): Promise => { + await runInstall({ignoreScripts: true}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true, ignoreScripts: false}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Flags don\'t match'); + }); +}); test.concurrent('when switching to --check-files install should rebuild integrity file', async (): Promise => { From 10abb6f65062ffae3bf2a875f58015da9cac85bd Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 25 Apr 2017 20:12:16 +0200 Subject: [PATCH 5/8] add test for integrity failed for linked modules --- __tests__/commands/check.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/__tests__/commands/check.js b/__tests__/commands/check.js index e202a7809e..62f94fd066 100644 --- a/__tests__/commands/check.js +++ b/__tests__/commands/check.js @@ -239,3 +239,21 @@ async (): Promise => { }); }); + +test.concurrent('--integrity should fail if integrity file have different linkedModules', async (): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter, install, getStdout): Promise => { + const integrityFilePath = path.join(config.cwd, 'node_modules', '.yarn-integrity'); + const integrityFile = JSON.parse(await fs.readFile(integrityFilePath)); + integrityFile.linkedModules.push("aLinkedModule"); + await fs.writeFile(integrityFilePath, JSON.stringify(integrityFile, null, 2)); + + let thrown = false; + try { + await checkCmd.run(config, reporter, {integrity: true}, []); + } catch (e) { + thrown = true; + } + expect(thrown).toEqual(true); + expect(getStdout()).toContain('Integrity check: Linked modules don\'t match'); + }); +}); From 59f0aeec65d2f09f3017b86a60341822adfa6f09 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 25 Apr 2017 20:13:07 +0200 Subject: [PATCH 6/8] delete warning for integrity failed for patterns because we return before if one pattern is missing and we never reach that code --- src/cli/commands/check.js | 1 - src/integrity-checker.js | 3 --- src/reporters/lang/en.js | 1 - 3 files changed, 5 deletions(-) diff --git a/src/cli/commands/check.js b/src/cli/commands/check.js index 45187d64f8..870d1c15f7 100644 --- a/src/cli/commands/check.js +++ b/src/cli/commands/check.js @@ -152,7 +152,6 @@ async function integrityHashCheck( 'FILES_MISSING': 'integrityFailedFilesMissing', 'LOCKFILE_DONT_MATCH': 'integrityLockfilesDontMatch', 'FLAGS_DONT_MATCH': 'integrityFlagsDontMatch', - 'PATTERNS_DONT_MATCH': 'integrityPatternsDontMatch', 'LINKED_MODULES_DONT_MATCH': 'integrityCheckLinkedModulesDontMatch', }; const integrityChecker = new InstallationIntegrityChecker(config); diff --git a/src/integrity-checker.js b/src/integrity-checker.js index ed00b2f07f..db03864dd2 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -183,9 +183,6 @@ export default class InstallationIntegrityChecker { if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) { return Promise.resolve('LINKED_MODULES_DONT_MATCH'); } - if (!compareSortedArrays(actual.topLevelPatters, expected.topLevelPatters)) { - return Promise.resolve('PATTERNS_DONT_MATCH'); - } if (!compareSortedArrays(actual.flags, expected.flags)) { return Promise.resolve('FLAGS_DONT_MATCH'); } diff --git a/src/reporters/lang/en.js b/src/reporters/lang/en.js index f99f086544..d8274679c9 100644 --- a/src/reporters/lang/en.js +++ b/src/reporters/lang/en.js @@ -256,7 +256,6 @@ const messages = { noIntegrityFile: 'Couldn\'t find an integrity file', integrityFailedExpectedIsNotAJSON: 'Integrity check: integrity file is not a json', integrityCheckLinkedModulesDontMatch: 'Integrity check: Linked modules don\'t match', - integrityPatternsDontMatch: 'Integrity check: Patterns don\'t match', integrityFlagsDontMatch: 'Integrity check: Flags don\'t match', integrityLockfilesDontMatch: 'Integrity check: Lock files don\'t match', integrityFailedFilesMissing: 'Integrity check: Files are missing', From 731e204d8e20f6976529ffb9b0bb2fa40e111751 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 25 Apr 2017 20:17:53 +0200 Subject: [PATCH 7/8] lint the code correctly --- __tests__/commands/check.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/__tests__/commands/check.js b/__tests__/commands/check.js index 62f94fd066..14ade7e5fe 100644 --- a/__tests__/commands/check.js +++ b/__tests__/commands/check.js @@ -241,10 +241,11 @@ async (): Promise => { }); test.concurrent('--integrity should fail if integrity file have different linkedModules', async (): Promise => { - await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter, install, getStdout): Promise => { + await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), + async (config, reporter, install, getStdout): Promise => { const integrityFilePath = path.join(config.cwd, 'node_modules', '.yarn-integrity'); const integrityFile = JSON.parse(await fs.readFile(integrityFilePath)); - integrityFile.linkedModules.push("aLinkedModule"); + integrityFile.linkedModules.push('aLinkedModule'); await fs.writeFile(integrityFilePath, JSON.stringify(integrityFile, null, 2)); let thrown = false; From 02b46c65012f3f03786fcf695050f608736972ac Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Wed, 26 Apr 2017 18:35:29 +0200 Subject: [PATCH 8/8] use flow enum, and remove every Promise.resolve in integrity-checker --- src/cli/commands/check.js | 10 ++-------- src/integrity-checker.js | 31 +++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/cli/commands/check.js b/src/cli/commands/check.js index 870d1c15f7..47c9b46cf0 100644 --- a/src/cli/commands/check.js +++ b/src/cli/commands/check.js @@ -3,6 +3,7 @@ import type Config from '../../config.js'; import {MessageError} from '../../errors.js'; import InstallationIntegrityChecker from '../../integrity-checker.js'; +import {integrityErrors} from '../../integrity-checker.js'; import Lockfile from '../../lockfile/wrapper.js'; import type {Reporter} from '../../reporters/index.js'; import * as fs from '../../util/fs.js'; @@ -147,13 +148,6 @@ async function integrityHashCheck( reporter.error(reporter.lang(msg, ...vars)); errCount++; } - const reasons = { - 'EXPECTED_IS_NOT_A_JSON': 'integrityFailedExpectedIsNotAJSON', - 'FILES_MISSING': 'integrityFailedFilesMissing', - 'LOCKFILE_DONT_MATCH': 'integrityLockfilesDontMatch', - 'FLAGS_DONT_MATCH': 'integrityFlagsDontMatch', - 'LINKED_MODULES_DONT_MATCH': 'integrityCheckLinkedModulesDontMatch', - }; const integrityChecker = new InstallationIntegrityChecker(config); const lockfile = await Lockfile.fromDirectory(config.cwd); @@ -170,7 +164,7 @@ async function integrityHashCheck( reportError('noIntegrityFile'); } if (match.integrityMatches === false) { - reporter.warn(reporter.lang(reasons[match.whyIntegrityMatchesFailed])); + reporter.warn(reporter.lang(integrityErrors[match.integrityError])); reportError('integrityCheckFailed'); } diff --git a/src/integrity-checker.js b/src/integrity-checker.js index db03864dd2..238c5df289 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -12,9 +12,20 @@ import type {InstallArtifacts} from './package-install-scripts.js'; const invariant = require('invariant'); const path = require('path'); +export const integrityErrors = { + EXPECTED_IS_NOT_A_JSON: 'integrityFailedExpectedIsNotAJSON', + FILES_MISSING: 'integrityFailedFilesMissing', + LOCKFILE_DONT_MATCH: 'integrityLockfilesDontMatch', + FLAGS_DONT_MATCH: 'integrityFlagsDontMatch', + LINKED_MODULES_DONT_MATCH: 'integrityCheckLinkedModulesDontMatch', +}; + +type IntegrityError = $Keys; + export type IntegrityCheckResult = { integrityFileMissing: boolean, integrityMatches?: boolean, + integrityError?: IntegrityError, missingPatterns: Array, }; @@ -176,24 +187,24 @@ export default class InstallationIntegrityChecker { actual: IntegrityFile, expected: ?IntegrityFile, checkFiles: boolean, - locationFolder: string): Promise { + locationFolder: string): Promise<'OK' | IntegrityError> { if (!expected) { - return Promise.resolve('EXPECTED_IS_NOT_A_JSON'); + return 'EXPECTED_IS_NOT_A_JSON'; } if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) { - return Promise.resolve('LINKED_MODULES_DONT_MATCH'); + return 'LINKED_MODULES_DONT_MATCH'; } if (!compareSortedArrays(actual.flags, expected.flags)) { - return Promise.resolve('FLAGS_DONT_MATCH'); + return 'FLAGS_DONT_MATCH'; } for (const key of Object.keys(actual.lockfileEntries)) { if (actual.lockfileEntries[key] !== expected.lockfileEntries[key]) { - return Promise.resolve('LOCKFILE_DONT_MATCH'); + return 'LOCKFILE_DONT_MATCH'; } } for (const key of Object.keys(expected.lockfileEntries)) { if (actual.lockfileEntries[key] !== expected.lockfileEntries[key]) { - return Promise.resolve('LOCKFILE_DONT_MATCH'); + return 'LOCKFILE_DONT_MATCH'; } } if (checkFiles) { @@ -202,18 +213,18 @@ export default class InstallationIntegrityChecker { // check and fail if there are file in node_modules after all. const actualFiles = await this._getFilesDeep(locationFolder); if (actualFiles.length > 0) { - return Promise.resolve('FILES_MISSING'); + return 'FILES_MISSING'; } } else { // TODO we may want to optimise this check by checking only for package.json files on very large trees for (const file of expected.files) { if (!await fs.exists(path.join(locationFolder, file))) { - return Promise.resolve('FILES_MISSING'); + return 'FILES_MISSING'; } } } } - return Promise.resolve('OK'); + return 'OK'; } async check( @@ -241,7 +252,7 @@ export default class InstallationIntegrityChecker { return { integrityFileMissing: false, integrityMatches: integrityMatches === 'OK', - whyIntegrityMatchesFailed: integrityMatches, + integrityError: integrityMatches === 'OK' ? undefined : integrityMatches, missingPatterns, }; }