feat(betterer ✨): no more weird completed then new tests (#1213)

.betterer.results

@@ -3,6 +3,10 @@
 // If this file contains merge conflicts, use `betterer merge` to automatically resolve them:
 // https://phenomnomnominal.github.io/betterer/docs/results-file/#merge
 //
+exports[`no hack comments`] = {
+  value: `{}`
+};
+
 exports[`no knip errors`] = {
   value: `{
     ".betterer.ts:194472954": [

goldens/api/betterer.api.md

@@ -420,6 +420,7 @@ export interface BettererRun {
     readonly expected: BettererResult | null;
     readonly filePaths: BettererFilePaths | null;
     readonly isNew: boolean;
+    readonly isObsolete: boolean;
     readonly isSkipped: boolean;
     readonly name: string;
 }
@@ -447,6 +448,7 @@ export interface BettererRunSummary extends BettererRun {
     readonly isComplete: boolean;
     readonly isExpired: boolean;
     readonly isFailed: boolean;
+    readonly isRemoved: boolean;
     readonly isSame: boolean;
     readonly isUpdated: boolean;
     readonly isWorse: boolean;
@@ -480,7 +482,9 @@ export interface BettererSuiteSummary extends BettererSuite {
     readonly expired: BettererRunSummaries;
     readonly failed: BettererRunSummaries;
     readonly new: BettererRunSummaries;
+    readonly obsolete: BettererRunSummaries;
     readonly ran: BettererRunSummaries;
+    readonly removed: BettererRunSummaries;
     readonly runSummaries: BettererRunSummaries;
     readonly same: BettererRunSummaries;
     readonly skipped: BettererRunSummaries;

packages/betterer/src/context/context.ts

@@ -90,7 +90,8 @@ export class BettererContextΩ implements BettererContext {
         const suiteSummary = await suite.run();
 
         if (!isRunOnce && !ci) {
-          await results.api.write(suiteSummary.result);
+          const suiteSummaryΩ = suiteSummary as BettererSuiteSummaryΩ;
+          await results.api.write(suiteSummaryΩ.result);
         }
 
         this._suiteSummaries = [...this._suiteSummaries, suiteSummary];

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

@@ -0,0 +1,50 @@
+import type { BettererResult } from '../results/index.js';
+import type { BettererRun, BettererRunSummary } from './types.js';
+
+import { BettererRunSummaryΩ } from './run-summary.js';
+
+export class BettererRunObsoleteΩ implements BettererRun {
+  public readonly expected: BettererResult;
+  public readonly isNew = false;
+  public readonly isObsolete = true;
+  public readonly isOnly = false;
+  public readonly isRemoved: boolean;
+  public readonly isSkipped = false;
+  public readonly filePaths = null;
+
+  constructor(
+    public readonly name: string,
+    public readonly baseline: BettererResult,
+    update = false
+  ) {
+    this.isRemoved = update;
+    this.expected = baseline;
+  }
+
+  public async run(): Promise<BettererRunSummary> {
+    return await Promise.resolve(
+      new BettererRunSummaryΩ({
+        baseline: this.baseline,
+        delta: null,
+        diff: null,
+        error: null,
+        expected: this.expected,
+        filePaths: this.filePaths,
+        isBetter: false,
+        isComplete: false,
+        isExpired: false,
+        isFailed: false,
+        isNew: false,
+        isObsolete: this.isObsolete,
+        isRemoved: this.isRemoved,
+        isSame: false,
+        isSkipped: false,
+        isUpdated: false,
+        isWorse: false,
+        name: this.name,
+        result: null,
+        timestamp: -Infinity
+      })
+    );
+  }
+}

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

@@ -15,6 +15,8 @@ export class BettererRunSummaryΩ implements BettererRunSummary {
   public readonly isExpired: boolean;
   public readonly isFailed: boolean;
   public readonly isNew: boolean;
+  public readonly isObsolete: boolean;
+  public readonly isRemoved: boolean;
   public readonly isSame: boolean;
   public readonly isSkipped: boolean;
   public readonly isUpdated: boolean;
@@ -35,6 +37,8 @@ export class BettererRunSummaryΩ implements BettererRunSummary {
     this.isExpired = summary.isExpired;
     this.isFailed = summary.isFailed;
     this.isNew = summary.isNew;
+    this.isObsolete = summary.isObsolete;
+    this.isRemoved = summary.isRemoved;
     this.isSame = summary.isSame;
     this.isSkipped = summary.isSkipped;
     this.isUpdated = summary.isUpdated;

packages/betterer/src/run/run.ts

@@ -14,7 +14,9 @@ import { BettererRunSummaryΩ } from './run-summary.js';
 
 export class BettererRunΩ implements BettererRun {
   public readonly isNew: boolean;
+  public readonly isObsolete = false;
   public readonly isOnly: boolean;
+  public readonly isRemoved = false;
   public readonly isSkipped: boolean;
   public readonly name: string;
 
@@ -95,8 +97,10 @@ export class BettererRunΩ implements BettererRun {
         isExpired: false,
         isFailed: true,
         isNew: this.isNew,
+        isObsolete: this.isObsolete,
+        isRemoved: this.isRemoved,
         isSame: false,
-        isSkipped: this.isSkipped || isFiltered,
+        isSkipped: false,
         isUpdated: false,
         isWorse: false,
         name: this.name,

packages/betterer/src/run/types.ts

@@ -93,6 +93,11 @@ export interface BettererRun {
    * will be set to `null`. The default reporter will show that this test is new.
    */
   readonly isNew: boolean;
+  /**
+   * When `true`, Betterer found a previous result for a test that is no longer relevant. Both `baseline` and `expected`
+   * will be set to previous result. The default reporter will show that this test is obsolete, and suggest that it can be removed.
+   */
+  readonly isObsolete: boolean;
   /**
    * When `true`, this test has been skipped and the test function will not run. The default
    * reporter will show that this test has been skipped.
@@ -203,6 +208,11 @@ export interface BettererRunSummary extends BettererRun {
    * reporter will show that this test has failed.
    */
   readonly isFailed: boolean;
+  /**
+   * When `true`, this test is "obsolete", due to a test being delete or renamed, but the `--update`
+   * option was used. is enabled. The previous result will be deleted from the {@link https://phenomnomnominal.github.io/betterer/docs/results-file | results file}.
+   */
+  readonly isRemoved: boolean;
   /**
    * When `true`, this test is "the same", based on the result of the `constraint` function.
    * The {@link https://phenomnomnominal.github.io/betterer/docs/results-file | results file} will

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

@@ -24,6 +24,8 @@ import { getGlobals, setGlobals } from '../globals.js';
 
 export class BettererWorkerRunΩ implements BettererRun {
   public readonly isNew: boolean;
+  public readonly isObsolete = false;
+  public readonly isRemoved = false;
   public readonly isSkipped: boolean;
   public readonly name: string;
 
@@ -208,6 +210,8 @@ export class BettererWorkerRunΩ implements BettererRun {
       isExpired,
       isFailed: false,
       isNew: this.isNew,
+      isObsolete: this.isObsolete,
+      isRemoved: this.isRemoved,
       isSame: comparison === BettererConstraintResult.same,
       isSkipped: !!isSkipped,
       isUpdated,

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

@@ -10,13 +10,19 @@ export class BettererSuiteSummaryΩ implements BettererSuiteSummary {
   public readonly result: BettererResultsSerialised;
 
   constructor(
-    private readonly _expectedTestNames: BettererTestNames,
     public readonly filePaths: BettererFilePaths,
     public readonly runs: BettererRuns,
     public readonly runSummaries: BettererRunSummaries
   ) {
     this.result = this._mergeResult();
   }
+  public get better(): BettererRunSummaries {
+    return this.runSummaries.filter((runSummary) => runSummary.isBetter);
+  }
+
+  public get changed(): BettererTestNames {
+    return this._getChanged();
+  }
 
   public get completed(): BettererRunSummaries {
     return this.runSummaries.filter((runSummary) => runSummary.isComplete);
@@ -26,10 +32,6 @@ export class BettererSuiteSummaryΩ implements BettererSuiteSummary {
     return this.runSummaries.filter((runSummary) => runSummary.isExpired);
   }
 
-  public get better(): BettererRunSummaries {
-    return this.runSummaries.filter((runSummary) => runSummary.isBetter);
-  }
-
   public get failed(): BettererRunSummaries {
     return this.runSummaries.filter((runSummary) => runSummary.isFailed);
   }
@@ -40,12 +42,22 @@ export class BettererSuiteSummaryΩ implements BettererSuiteSummary {
     );
   }
 
+  public get obsolete(): BettererRunSummaries {
+    return this.runSummaries.filter((runSummary) => runSummary.isObsolete && !runSummary.isRemoved);
+  }
+
   public get ran(): BettererRunSummaries {
-    return this.runSummaries.filter((runSummary) => !(runSummary.isSkipped || runSummary.isFailed));
+    return this.runSummaries.filter(
+      (runSummary) => !(runSummary.isSkipped || runSummary.isFailed || runSummary.isObsolete || runSummary.isRemoved)
+    );
+  }
+
+  public get removed(): BettererRunSummaries {
+    return this.runSummaries.filter((runSummary) => runSummary.isRemoved);
   }
 
   public get same(): BettererRunSummaries {
-    return this.runSummaries.filter((runSummary) => runSummary.isSame);
+    return this.runSummaries.filter((runSummary) => runSummary.isSame && !runSummary.isComplete);
   }
 
   public get skipped(): BettererRunSummaries {
@@ -57,17 +69,11 @@ export class BettererSuiteSummaryΩ implements BettererSuiteSummary {
   }
 
   public get worse(): BettererRunSummaries {
-    return this.runSummaries.filter((runSummary) => runSummary.isWorse);
-  }
-
-  public get changed(): BettererTestNames {
-    return this._getChanged();
+    return this.runSummaries.filter((runSummary) => runSummary.isWorse && !runSummary.isUpdated);
   }
 
   private _getChanged(): BettererTestNames {
-    const missingRunNames = this._expectedTestNames.filter(
-      (name) => !this.runSummaries.find((runSummary) => runSummary.name === name)
-    );
+    const obsoleteRunNames = this.obsolete.map((runSummary) => runSummary.name);
     const notFailedOrSkipped = this.runSummaries.filter((runSummary) => !runSummary.isFailed && !runSummary.isSkipped);
     const changedRuns = notFailedOrSkipped
       .filter((runSummary) => !runSummary.isNew)
@@ -79,23 +85,23 @@ export class BettererSuiteSummaryΩ implements BettererSuiteSummary {
       });
     const newRuns = notFailedOrSkipped.filter((runSummary) => runSummary.isNew && !runSummary.isComplete);
     const newOrChangedRunNames = [...changedRuns, ...newRuns].map((runSummary) => runSummary.name);
-    return [...missingRunNames, ...newOrChangedRunNames];
+    return [...obsoleteRunNames, ...newOrChangedRunNames];
   }
 
   private _mergeResult(): BettererResultsSerialised {
     return this.runSummaries.reduce<BettererResultsSerialised>((results, runSummary) => {
-      const { isComplete, isFailed, isSkipped, isNew, isWorse } = runSummary;
+      const { isFailed, isSkipped, isNew, isObsolete, isRemoved, isWorse } = runSummary;
       const isSkippedOrFailed = isSkipped || isFailed;
-      if (isComplete || (isSkippedOrFailed && isNew)) {
+      if (isRemoved || (isSkippedOrFailed && isNew)) {
         return results;
       }
       const { expected, name, result } = runSummary;
-      if ((isSkippedOrFailed && !isNew) || isWorse) {
-        invariantΔ(expected, 'Test is not _new_, or _worse_ so it must have an expected result!');
+      if ((isSkippedOrFailed && !isNew) || isWorse || isObsolete) {
+        invariantΔ(expected, 'Test has successfully run in the past so it must have an expected result!');
         results[name] = { value: expected.printed };
         return results;
       }
-      invariantΔ(result, 'Test is not _completed_, _skipped_ or _failed_ so it must have a new result!');
+      invariantΔ(result, 'Test has successfully run so it must have a new result!');
       results[name] = { value: result.printed };
       return results;
     }, {});

packages/betterer/src/suite/suite.ts

@@ -3,14 +3,16 @@ import type { BettererError } from '@betterer/errors';
 import type { BettererFilePaths } from '../fs/index.js';
 import type { BettererReporterΩ } from '../reporters/index.js';
 import type { BettererReporterRun, BettererRunSummary, BettererRuns } from '../run/index.js';
-import type { BettererSuite } from './types.js';
+import type { BettererSuite, BettererSuiteSummary } from './types.js';
 
 import { invariantΔ } from '@betterer/errors';
 
 import { defer } from '../utils.js';
 import { BettererSuiteSummaryΩ } from './suite-summary.js';
 import { BettererRunΩ } from '../run/index.js';
 import { getGlobals } from '../globals.js';
+import { BettererRunObsoleteΩ } from '../run/run-obsolete.js';
+import { BettererResultΩ } from '../results/result.js';
 
 const NEGATIVE_FILTER_TOKEN = '!';
 
@@ -21,20 +23,32 @@ export class BettererSuiteΩ implements BettererSuite {
   ) {}
 
   public static async create(filePaths: BettererFilePaths): Promise<BettererSuiteΩ> {
-    const { config, testMetaLoader } = getGlobals();
-    const { configPaths } = config;
+    const { config, results, testMetaLoader } = getGlobals();
+    const { configPaths, update } = config;
+
+    const expectedTestNames = await results.api.getExpectedTestNames();
 
     const testsMeta = await testMetaLoader.api.loadTestsMeta(configPaths);
     const runsΩ = await Promise.all(
       testsMeta.map(async (testMeta) => {
         return await BettererRunΩ.create(testMeta, filePaths);
       })
     );
+    const testNames = testsMeta.map((testMeta) => testMeta.name);
+    const obsoleteTestNames = expectedTestNames.filter((expectedTestName) => !testNames.includes(expectedTestName));
+
+    const obsoleteRuns = await Promise.all(
+      obsoleteTestNames.map(async (testName) => {
+        const baselineJSON = await results.api.getBaseline(testName);
+        const baseline = new BettererResultΩ(JSON.parse(baselineJSON), baselineJSON);
+        return new BettererRunObsoleteΩ(testName, baseline, update);
+      })
+    );
 
-    return new BettererSuiteΩ(filePaths, runsΩ);
+    return new BettererSuiteΩ(filePaths, [...obsoleteRuns, ...runsΩ]);
   }
 
-  public async run(): Promise<BettererSuiteSummaryΩ> {
+  public async run(): Promise<BettererSuiteSummary> {
     const hasOnly = !!this.runs.find((run) => {
       const runΩ = run as BettererRunΩ;
       return runΩ.isOnly;
@@ -109,8 +123,6 @@ export class BettererSuiteΩ implements BettererSuite {
       })
     );
 
-    const { results } = getGlobals();
-    const expectedTestNames = await results.api.getExpectedTestNames();
-    return new BettererSuiteSummaryΩ(expectedTestNames, this.filePaths, this.runs, runSummaries);
+    return new BettererSuiteSummaryΩ(this.filePaths, this.runs, runSummaries);
   }
 }

packages/betterer/src/suite/types.ts

@@ -109,11 +109,21 @@ export interface BettererSuiteSummary extends BettererSuite {
    * for each test that was run for the first time.
    */
   readonly new: BettererRunSummaries;
+  /**
+   * An array containing a {@link @betterer/betterer#BettererRunSummary | `BettererRunSummary`}
+   * for each test that has a previous saved result but is no longer defined.
+   */
+  readonly obsolete: BettererRunSummaries;
   /**
    * An array containing a {@link @betterer/betterer#BettererRunSummary | `BettererRunSummary`}
    * for each test that didn't fail and wasn't skipped.
    */
   readonly ran: BettererRunSummaries;
+  /**
+   * An array containing a {@link @betterer/betterer#BettererRunSummary | `BettererRunSummary`}
+   * for each test that is obsolete, but the `--update` option was enabled.
+   */
+  readonly removed: BettererRunSummaries;
   /**
    * An array containing a {@link @betterer/betterer#BettererRunSummary | `BettererRunSummary`}
    * for each test that stayed the same.

packages/betterer/src/test/file-test/printer.ts

@@ -3,8 +3,12 @@ import type { BettererFileTestResultSerialised } from './types.js';
 import { isKey } from './serialiser.js';
 
 export function printer(serialised: BettererFileTestResultSerialised): string {
+  const keys = Object.keys(serialised);
+  if (keys.length === 0) {
+    return '{}';
+  }
   let printed = '{\n';
-  Object.keys(serialised)
+  keys
     .sort()
     .filter(isKey)
     .forEach((filePath, index) => {