feat(betterer ✨): explain what changed when there is a diff in ci mode (

#827)

goldens/api/@betterer/betterer.d.ts

@@ -292,9 +292,7 @@ export declare type BettererSuiteSummaries = ReadonlyArray<BettererSuiteSummary>
 
 export declare type BettererSuiteSummary = BettererSuite & {
     readonly runs: BettererRunSummaries;
-    readonly result: string;
-    readonly expected: string | null;
-    readonly unexpectedDiff: boolean;
+    readonly changed: BettererRunNames;
     readonly better: BettererRunSummaries;
     readonly completed: BettererRunSummaries;
     readonly expired: BettererRunSummaries;

goldens/api/@betterer/cli.d.ts

@@ -72,6 +72,6 @@ export declare function initΔ(cwd: string, argv: BettererCLIArguments): Promise
 
 export declare function precommitΔ(cwd: string, argv: BettererCLIArguments): Promise<BettererSuiteSummary>;
 
-export declare function startΔ(cwd: string, argv: BettererCLIArguments): Promise<BettererSuiteSummary>;
+export declare function startΔ(cwd: string, argv: BettererCLIArguments, ci?: boolean): Promise<BettererSuiteSummary>;
 
 export declare function watchΔ(cwd: string, argv: BettererCLIArguments): Promise<void>;

jest.config.js

@@ -12,5 +12,6 @@ module.exports = {
   coverageDirectory: '<rootDir>/reports/coverage',
   preset: 'ts-jest',
   testEnvironment: 'node',
-  testRegex: '.*\\.spec\\.ts$'
+  testRegex: '.*\\.spec\\.ts$',
+  watchPathIgnorePatterns: ['<rootDir>/fixtures', '<rootDir>/packages/[^/]+/src']
 };

packages/betterer/src/context/context.ts

@@ -41,14 +41,10 @@ export class BettererContextΩ implements BettererContext {
         await this.reporter.contextEnd(contextSummary);
 
         const suiteSummaryΩ = contextSummary.lastSuite;
-        if (suiteSummaryΩ.shouldWrite) {
-          await this.results.write(suiteSummaryΩ.result);
-
-          await this.versionControl.writeCache();
-          if (this.config.precommit) {
-            await this.versionControl.add(this.config.resultsPath);
-          }
+        if (!this.config.ci) {
+          await this.results.write(suiteSummaryΩ, this.config.precommit);
         }
+
         return contextSummary;
       },
       error: async (error: BettererError): Promise<void> => {

packages/betterer/src/globals.ts

@@ -21,7 +21,7 @@ export async function createGlobals(options: unknown = {}): Promise<BettererGlob
       reporter = loadReporters(reporters, cwd);
     }
     await registerExtensions(config);
-    const results = new BettererResultsΩ(config);
+    const results = await BettererResultsΩ.create(config.resultsPath, versionControl);
     return { config, reporter, results, versionControl };
   } catch (error) {
     await reporter.configError(options, error);
@@ -43,7 +43,7 @@ export async function createWorkerGlobals(
   await registerExtensions(config);
 
   const reporter = loadReporters([]);
-  const results = new BettererResultsΩ(config);
+  const results = await BettererResultsΩ.create(config.resultsPath, versionControl);
 
   return { config, reporter, results, versionControl };
 }

packages/betterer/src/results/results.ts

@@ -1,33 +1,45 @@
 import assert from 'assert';
-import { BettererConfig } from '../config';
 
-import { forceRelativePaths, read, write } from '../fs';
+import { read, write, BettererVersionControlWorker } from '../fs';
 import { BettererRunSummaries } from '../run';
+import { BettererRunNames } from '../run/types';
+import { BettererSuiteSummary } from '../suite';
 import { parse } from './parser';
 import { print } from './printer';
 import { BettererExpectedResults } from './types';
 
 const RESULTS_HEADER = `// BETTERER RESULTS V2.`;
 
 export class BettererResultsΩ {
-  private _baseline: BettererExpectedResults | null = null;
-  private _expected: BettererExpectedResults | null = null;
-  private _resultsPath: string;
+  private constructor(
+    private _resultsPath: string,
+    private _versionControl: BettererVersionControlWorker,
+    private _baseline: BettererExpectedResults,
+    private _expected: BettererExpectedResults
+  ) {}
 
-  constructor(private _config: BettererConfig) {
-    this._resultsPath = this._config.resultsPath;
+  public static async create(
+    resultsPath: string,
+    versionControl: BettererVersionControlWorker
+  ): Promise<BettererResultsΩ> {
+    const baseline = await parse(resultsPath);
+    return new BettererResultsΩ(resultsPath, versionControl, baseline, baseline);
   }
 
-  public async sync(): Promise<void> {
-    this._expected = await parse(this._resultsPath);
-    if (!this._baseline) {
-      this._baseline = this._expected;
-    }
+  public getChanged(runSummaries: BettererRunSummaries): BettererRunNames {
+    const missingRuns = Object.keys(this._expected).filter(
+      (name) => !runSummaries.find((runSummary) => runSummary.name === name)
+    );
+    const changedRuns = runSummaries
+      .filter((runSummary) => !runSummary.isNew && !runSummary.isFailed && !runSummary.isSkipped)
+      .filter((runSummary) => runSummary.printed !== this._expected[runSummary.name].value)
+      .map((runSummary) => runSummary.name);
+    const newRuns = runSummaries.filter((runSummary) => runSummary.isNew).map((runSummary) => runSummary.name);
+    const worseRuns = runSummaries.filter((runSummary) => runSummary.isWorse).map((runSummary) => runSummary.name);
+    return [...missingRuns, ...changedRuns, ...newRuns, ...worseRuns];
   }
 
   public getExpected(name: string): [string, string] {
-    assert(this._baseline);
-    assert(this._expected);
     const baseline = this._getResult(name, this._baseline);
     const expected = this._getResult(name, this._expected) || baseline;
     return [baseline, expected];
@@ -37,19 +49,21 @@ export class BettererResultsΩ {
     return Object.hasOwnProperty.call(this._expected, name);
   }
 
-  public read(): Promise<string | null> {
-    return read(this._resultsPath);
-  }
-
-  public print(runs: BettererRunSummaries): string {
-    const toPrint = runs.filter((run) => run.printed != null);
-    const printedResults = toPrint.map((run) => print(run.name, run.printed as string));
-    const printed = [RESULTS_HEADER, ...printedResults].join('');
-    return forceRelativePaths(printed, this._resultsPath);
+  public async sync(): Promise<void> {
+    this._expected = await parse(this._resultsPath);
   }
 
-  public write(printed: string): Promise<void> {
-    return write(printed, this._resultsPath);
+  public async write(suiteSummary: BettererSuiteSummary, precommit: boolean): Promise<void> {
+    const expected = await read(this._resultsPath);
+    const result = this._print(suiteSummary.runs);
+    const shouldWrite = expected !== result;
+    if (shouldWrite) {
+      await write(result, this._resultsPath);
+      await this._versionControl.writeCache();
+      if (precommit) {
+        await this._versionControl.add(this._resultsPath);
+      }
+    }
   }
 
   private _getResult(name: string, expectedResults: BettererExpectedResults): string {
@@ -58,4 +72,10 @@ export class BettererResultsΩ {
     const { value } = expectedResults[name];
     return value;
   }
+
+  private _print(runSummaries: BettererRunSummaries): string {
+    const toPrint = runSummaries.filter((runSummary) => runSummary.printed != null);
+    const printedResults = toPrint.map((runSummary) => print(runSummary.name, runSummary.printed as string));
+    return [RESULTS_HEADER, ...printedResults].join('');
+  }
 }

packages/betterer/src/run/index.ts

@@ -1,5 +1,12 @@
 export { BettererRunΩ, BettererRunsΩ } from './run';
 export { BettererRunWorkerPoolΩ } from './run-worker-pool';
-export { BettererRun, BettererRuns, BettererReporterRun, BettererRunSummary, BettererRunSummaries } from './types';
+export {
+  BettererRun,
+  BettererRuns,
+  BettererRunNames,
+  BettererReporterRun,
+  BettererRunSummary,
+  BettererRunSummaries
+} from './types';
 export { createWorkerConfig } from './worker-run-config';
 export { BettererWorkerRunΩ } from './worker-run';

packages/betterer/src/run/run-summary.ts

@@ -46,7 +46,7 @@ export class BettererRunSummaryΩ implements BettererRunSummary {
     this.filePaths = run.filePaths;
     this.isBetter = status === BettererRunStatus.better;
     this.isFailed = status === BettererRunStatus.failed;
-    this.isNew = run.isNew;
+    this.isNew = status === BettererRunStatus.new;
     this.isSame = status === BettererRunStatus.same;
     this.isSkipped = status === BettererRunStatus.skipped;
     this.isUpdated = status === BettererRunStatus.update;

packages/betterer/src/run/worker-run.ts

@@ -3,7 +3,7 @@ import { BettererError } from '@betterer/errors';
 import assert from 'assert';
 
 import { BettererConfig } from '../config';
-import { BettererFilePaths, BettererVersionControlWorker } from '../fs';
+import { BettererFilePaths, BettererVersionControlWorker, forceRelativePaths } from '../fs';
 import { createWorkerGlobals } from '../globals';
 import { BettererResult, BettererResultΩ } from '../results';
 import { BettererDiff, BettererTestConfig, BettererTestMeta, isBettererFileTest, loadTestMeta } from '../test';
@@ -167,7 +167,7 @@ export class BettererWorkerRunΩ implements BettererRun {
       if (shouldPrint) {
         const toPrint = isFailed || isSkipped || isWorse ? this.expected : (result as BettererResult);
         const toPrintSerialised = this.test.serialiser.serialise(toPrint.value, config.resultsPath);
-        printed = await this.test.printer(toPrintSerialised);
+        printed = forceRelativePaths(await this.test.printer(toPrintSerialised), config.resultsPath);
       }
 
       return new BettererRunSummaryΩ(

packages/betterer/src/suite/suite-summary.ts

@@ -1,25 +1,13 @@
 import { BettererFilePaths } from '../fs';
-import { BettererRunSummaries } from '../run';
-import { BettererSuite, BettererSuiteSummary } from './types';
+import { BettererRunNames, BettererRunSummaries } from '../run';
+import { BettererSuiteSummary } from './types';
 
 export class BettererSuiteSummaryΩ implements BettererSuiteSummary {
-  public readonly filePaths: BettererFilePaths;
-  public readonly unexpectedDiff: boolean;
-  public readonly shouldWrite: boolean;
-
   constructor(
-    suite: BettererSuite,
+    public readonly filePaths: BettererFilePaths,
     public readonly runs: BettererRunSummaries,
-    public readonly result: string,
-    public readonly expected: string | null,
-    ci: boolean
-  ) {
-    this.filePaths = suite.filePaths;
-    const hasDiff = !!this.expected && this.expected !== this.result;
-    this.unexpectedDiff = hasDiff && ci;
-    const expectedDiff = hasDiff && !ci;
-    this.shouldWrite = !this.expected || expectedDiff;
-  }
+    public readonly changed: BettererRunNames
+  ) {}
 
   public get completed(): BettererRunSummaries {
     return this.runs.filter((run) => run.isComplete);

packages/betterer/src/suite/suite.ts

@@ -36,9 +36,8 @@ export class BettererSuiteΩ implements BettererSuite {
     const reportSuiteStart = this._reporter.suiteStart(this, runsLifecycle.promise);
     try {
       const runSummaries = await this._runTests(runLifecycles);
-      const result = this._results.print(runSummaries);
-      const expected = await this._results.read();
-      const suiteSummary = new BettererSuiteSummaryΩ(this, runSummaries, result, expected, this._config.ci);
+      const changed = this._results.getChanged(runSummaries);
+      const suiteSummary = new BettererSuiteSummaryΩ(this.filePaths, runSummaries, changed);
       runsLifecycle.resolve(suiteSummary);
       await reportSuiteStart;
       await this._reporter.suiteEnd(suiteSummary);

packages/betterer/src/suite/types.ts

@@ -1,5 +1,5 @@
 import { BettererFilePaths } from '../fs';
-import { BettererRuns, BettererRunSummaries } from '../run';
+import { BettererRunNames, BettererRuns, BettererRunSummaries } from '../run';
 
 export type BettererSuite = {
   readonly filePaths: BettererFilePaths;
@@ -8,9 +8,7 @@ export type BettererSuite = {
 
 export type BettererSuiteSummary = BettererSuite & {
   readonly runs: BettererRunSummaries;
-  readonly result: string;
-  readonly expected: string | null;
-  readonly unexpectedDiff: boolean;
+  readonly changed: BettererRunNames;
 
   readonly better: BettererRunSummaries;
   readonly completed: BettererRunSummaries;

packages/cli/bin/betterer-ci

@@ -2,7 +2,7 @@
 require('../dist/ci')
   .ciΔ(process.cwd(), process.argv)
   .then(function (suiteSummary) {
-    process.exitCode = suiteSummary.unexpectedDiff || suiteSummary.worse.length !== 0 ? 1 : 0;
+    process.exitCode = suiteSummary.changed.length ? 1 : 0;
   })
   .catch(function () {
     process.exitCode = 1;

packages/cli/src/start.ts

@@ -5,8 +5,12 @@ import { cliOptions } from './options';
 import { BettererCLIArguments } from './types';
 
 /** @internal Definitely not stable! Please don't use! */
-export function startΔ(cwd: string, argv: BettererCLIArguments): Promise<BettererSuiteSummary> {
-  if (process.env.CI) {
+export function startΔ(
+  cwd: string,
+  argv: BettererCLIArguments,
+  ci = process.env.CI === 'true'
+): Promise<BettererSuiteSummary> {
+  if (ci) {
     return ciΔ(cwd, argv);
   }
 

packages/reporter/src/components/Reporter.tsx

@@ -13,7 +13,7 @@ export const Reporter: FC<BettererReporterState> = function Reporter(props: Bett
   const ReporterComponent = context.config.watch ? WatchReporter : DefaultReporter;
 
   return (
-    <Box flexDirection="column" paddingBottom={1}>
+    <Box flexDirection="column">
       <BettererLogo></BettererLogo>
       <ReporterComponent {...props} />
     </Box>

packages/reporter/src/components/runs/SuiteSummary.tsx

@@ -1,7 +1,6 @@
 import React, { FC, memo } from 'react';
 
 import { BettererContext, BettererSuiteSummary } from '@betterer/betterer';
-import { diffΔ } from '@betterer/logger';
 import { Box, Text, TextProps } from 'ink';
 
 import {
@@ -15,7 +14,8 @@ import {
   testSkipped,
   testUpdated,
   testWorse,
-  unexpectedDiff,
+  unexpectedChanges,
+  unexpectedChangesInstructions,
   updateInstructions
 } from '../../messages';
 import { quote } from '../../utils';
@@ -27,16 +27,16 @@ export type SuiteSummaryProps = {
 
 const TEXT_COLOURS: Record<string, TextProps['color']> = {
   better: 'greenBright',
+  changed: 'red',
   checked: 'gray',
   completed: 'greenBright',
-  diff: 'red',
   expired: 'brightRed',
   failed: 'brightRed',
   new: 'gray',
   obsolete: 'brightRed',
   same: 'brightYellow',
   skipped: 'brightYellow',
-  updated: 'gray',
+  updated: 'white',
   worse: 'red'
 };
 
@@ -80,10 +80,15 @@ export const SuiteSummary: FC<SuiteSummaryProps> = memo(function SuiteSummary({
           </>
         ) : null}
       </Box>
-      {suiteSummary.unexpectedDiff ? (
+      {context.config.ci && suiteSummary.changed.length ? (
         <Box flexDirection="column" paddingBottom={1}>
-          <Text color={TEXT_COLOURS.diff}>{unexpectedDiff()}</Text>
-          <Text>{diffΔ(suiteSummary.expected, suiteSummary.result)}</Text>
+          <Text color={TEXT_COLOURS.changed}>{unexpectedChanges()}</Text>
+          <Box flexDirection="column" padding={1}>
+            {suiteSummary.changed.map((name) => (
+              <Text key={name}>"{name}"</Text>
+            ))}
+          </Box>
+          <Text color={TEXT_COLOURS.changed}>{unexpectedChangesInstructions()}</Text>
         </Box>
       ) : null}
     </>

packages/reporter/src/messages.ts

@@ -33,11 +33,15 @@ export function testWorse(context: string, delta = ''): string {
 }
 
 export function updateInstructions(): string {
-  return `You should try to fix the new issues! As a last resort, you can run \`betterer --update\` to force an update of the results file. 🆙`;
+  return 'You should try to fix the new issues! As a last resort, you can run `betterer --update` to force an update of the results file. 🆙';
 }
 
-export function unexpectedDiff(): string {
-  return 'Unexpected diff found:';
+export function unexpectedChanges(): string {
+  return 'Unexpected changes detected in these tests while running in CI mode:';
+}
+
+export function unexpectedChangesInstructions(): string {
+  return 'You should make sure the results file is up-to-date before committing! You might want to run `betterer precommit` in a commit hook. 💍';
 }
 
 export function filesChecking(files: number): string {