extract the reporter out of integrity checker

__tests__/commands/_helpers.js

@@ -65,7 +65,7 @@ export async function run<T, R>(
   args: Array<string>,
   flags: Object,
   name: string | { source: string, cwd: string },
-  checkInstalled: ?(config: Config, reporter: R, install: T) => ?Promise<void>,
+  checkInstalled: ?(config: Config, reporter: R, install: T, getStdout: () => string) => ?Promise<void>,
   beforeInstall: ?(cwd: string) => ?Promise<void>,
 ): Promise<void> {
   let out = '';
@@ -131,7 +131,7 @@ export async function run<T, R>(
     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}`);

__tests__/commands/check.js

@@ -21,7 +21,7 @@ const runCheck = buildRun.bind(
   },
 );
 
-test.concurrent('--verify-tree should report wrong version ', async (): Promise<void> => {
+test.concurrent('--verify-tree should report wrong version', async (): Promise<void> => {
   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<void> => {
+test.concurrent('--verify-tree should report missing dependency', async (): Promise<void> => {
   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<void> => {
+  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise<void> => {
+    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<void> => {
+  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'),
+  async (config, reporter, install, getStdout): Promise<void> => {
+    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<void> => {
   await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise<void> => {
     let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
@@ -97,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<void> => {
-  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise<void> => {
+  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'),
+  async (config, reporter, install, getStdout): Promise<void> => {
     let lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
     lockfile += `\nxtend@^4.0.0:
   version "4.0.1"
@@ -111,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<void> => {
-  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'), async (config, reporter): Promise<void> => {
+  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'),
+  async (config, reporter, install, getStdout): Promise<void> => {
     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');
@@ -128,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<void> => {
   await runInstall({checkFiles: true}, path.join('..', 'check', 'integrity-lock-check'),
-  async (config, reporter): Promise<void> => {
+  async (config, reporter, install, getStdout): Promise<void> => {
     await fs.unlink(path.join(config.cwd, 'node_modules', 'left-pad', 'index.js'));
 
     let thrown = false;
@@ -144,22 +178,23 @@ async (): Promise<void> => {
       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<void> => {
-    await runInstall({ignoreScripts: true}, path.join('..', 'check', 'integrity-lock-check'),
-      async (config, reporter): Promise<void> => {
-        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<void> => {
+  await runInstall({ignoreScripts: true}, path.join('..', 'check', 'integrity-lock-check'),
+    async (config, reporter, install, getStdout): Promise<void> => {
+      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<void> => {
@@ -204,3 +239,22 @@ async (): Promise<void> => {
 
   });
 });
+
+test.concurrent('--integrity should fail if integrity file have different linkedModules', async (): Promise<void> => {
+  await runInstall({}, path.join('..', 'check', 'integrity-lock-check'),
+  async (config, reporter, install, getStdout): Promise<void> => {
+    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');
+  });
+});

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,7 +148,7 @@ async function integrityHashCheck(
     reporter.error(reporter.lang(msg, ...vars));
     errCount++;
   }
-  const integrityChecker = new InstallationIntegrityChecker(config, reporter);
+  const integrityChecker = new InstallationIntegrityChecker(config);
 
   const lockfile = await Lockfile.fromDirectory(config.cwd);
   const install = new Install(flags, config, reporter, lockfile);
@@ -162,7 +163,8 @@ async function integrityHashCheck(
   if (match.integrityFileMissing) {
     reportError('noIntegrityFile');
   }
-  if (!match.integrityMatches) {
+  if (match.integrityMatches === false) {
+    reporter.warn(reporter.lang(integrityErrors[match.integrityError]));
     reportError('integrityCheckFailed');
   }
 

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);

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';
@@ -13,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<typeof integrityErrors>;
+
 export type IntegrityCheckResult = {
   integrityFileMissing: boolean,
   integrityMatches?: boolean,
+  integrityError?: IntegrityError,
   missingPatterns: Array<string>,
 };
 
@@ -47,15 +57,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 +173,58 @@ export default class InstallationIntegrityChecker {
     return result;
   }
 
-  _compareIntegrityFiles(actual: IntegrityFile, expected: IntegrityFile): boolean {
-    if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) {
-      this.reporter.warn(this.reporter.lang('integrityCheckLinkedModulesDontMatch'));
-      return false;
+  async _getIntegrityFile(locationPath: string): Promise<?IntegrityFile> {
+    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<'OK' | IntegrityError> {
+    if (!expected) {
+      return 'EXPECTED_IS_NOT_A_JSON';
     }
-    if (!compareSortedArrays(actual.topLevelPatters, expected.topLevelPatters)) {
-      this.reporter.warn(this.reporter.lang('integrityPatternsDontMatch'));
-      return false;
+    if (!compareSortedArrays(actual.linkedModules, expected.linkedModules)) {
+      return 'LINKED_MODULES_DONT_MATCH';
     }
     if (!compareSortedArrays(actual.flags, expected.flags)) {
-      this.reporter.warn(this.reporter.lang('integrityFlagsDontMatch'));
-      return false;
+      return '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 '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 '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 '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 'FILES_MISSING';
+          }
+        }
       }
     }
-    return true;
+    return 'OK';
   }
 
   async check(
@@ -214,41 +246,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',
+      integrityError: integrityMatches === 'OK' ? undefined : integrityMatches,
       missingPatterns,
     };
   }
@@ -295,5 +299,4 @@ export default class InstallationIntegrityChecker {
       await fs.unlink(loc.locationPath);
     }
   }
-
 }

src/reporters/lang/en.js

@@ -254,8 +254,8 @@ 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',
   integrityLockfilesDontMatch: 'Integrity check: Lock files don\'t match',
   integrityFailedFilesMissing: 'Integrity check: Files are missing',