Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions goldens/api/@betterer/betterer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion goldens/api/@betterer/cli.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
3 changes: 2 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']
};
10 changes: 3 additions & 7 deletions packages/betterer/src/context/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> => {
Expand Down
4 changes: 2 additions & 2 deletions packages/betterer/src/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 };
}
70 changes: 45 additions & 25 deletions packages/betterer/src/results/results.ts
Original file line number Diff line number Diff line change
@@ -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];
Expand All @@ -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 {
Expand All @@ -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('');
}
}
9 changes: 8 additions & 1 deletion packages/betterer/src/run/index.ts
Original file line number Diff line number Diff line change
@@ -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';
2 changes: 1 addition & 1 deletion packages/betterer/src/run/run-summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/betterer/src/run/worker-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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Ω(
Expand Down
22 changes: 5 additions & 17 deletions packages/betterer/src/suite/suite-summary.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
5 changes: 2 additions & 3 deletions packages/betterer/src/suite/suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions packages/betterer/src/suite/types.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/bin/betterer-ci
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/reporter/src/components/Reporter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
19 changes: 12 additions & 7 deletions packages/reporter/src/components/runs/SuiteSummary.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -15,7 +14,8 @@ import {
testSkipped,
testUpdated,
testWorse,
unexpectedDiff,
unexpectedChanges,
unexpectedChangesInstructions,
updateInstructions
} from '../../messages';
import { quote } from '../../utils';
Expand All @@ -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'
};

Expand Down Expand Up @@ -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}
</>
Expand Down
10 changes: 7 additions & 3 deletions packages/reporter/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading