Skip to content

Commit

Permalink
add new flags for comparing warning and error diffs (#1888)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorskees authored Mar 20, 2023
1 parent db3e244 commit 778aefa
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 14 deletions.
22 changes: 22 additions & 0 deletions lib/cli-args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ export interface CliArgs {
interactive: boolean;
testDirs: string[];
todoMode: TodoMode;
trimErrors: boolean;
skipWarning: boolean;
ignoreErrorDiffs: boolean;
}

const implArgs: Record<string, string[]> = {
Expand Down Expand Up @@ -99,6 +102,22 @@ export async function parseArgs(
type: 'boolean',
default: false,
})
.options('trim-errors', {
description: 'Only compare the message for errors',
type: 'boolean',
default: false,
})
.options('ignore-warning-diffs', {
description: 'Ignore diffs in warnings',
type: 'boolean',
default: false,
})
.options('ignore-error-diffs', {
description:
'Ignore the contents of error messages. I.e. only validate that an error was thrown.',
type: 'boolean',
default: false,
})
.parseSync();

const root = path.resolve(process.cwd(), argv['root-path']);
Expand All @@ -113,6 +132,9 @@ export async function parseArgs(
: argv['probe-todo']
? 'probe'
: undefined,
trimErrors: argv['trim-errors'],
skipWarning: argv['ignore-warning-diffs'],
ignoreErrorDiffs: argv['ignore-error-diffs'],
};
args.impl = argv.dart ? 'dart-sass' : argv.impl!;
let cmdArgs = implArgs[args.impl] ?? [];
Expand Down
18 changes: 12 additions & 6 deletions lib/test-case/compare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,18 @@ function getDiff(
return createPatch(filename, expected, actual, 'expected', 'actual');
}

interface CompareOptions {
export interface CompareOptions {
/**
* if true, errors and warnings will be trimmed
* so only the messages are compared and not line information
*/
trimErrors?: boolean;
/** If true, skip warning checks */
skipWarning?: boolean;
/**
* If true, error messages and line information won't be compared
*/
ignoreErrorDiffs?: boolean;
}

/**
Expand All @@ -77,7 +81,7 @@ interface CompareOptions {
export function compareResults(
expected: ExpectedSassResult,
actual: SassResult,
{skipWarning, trimErrors}: CompareOptions
{skipWarning, trimErrors, ignoreErrorDiffs}: CompareOptions
): TestResult {
if (expected.isSuccess === null) {
return failures.MissingOutput();
Expand Down Expand Up @@ -107,10 +111,12 @@ export function compareResults(
if (actual.isSuccess) {
return failures.UnexpectedSuccess();
}
const normalizer = trimErrors ? extractErrorMessage : normalizeOutput;
const diff = getDiff('error', expected.error, actual.error, normalizer);
if (diff) {
return failures.ErrorDifference(diff);
if (!ignoreErrorDiffs) {
const normalizer = trimErrors ? extractErrorMessage : normalizeOutput;
const diff = getDiff('error', expected.error, actual.error, normalizer);
if (diff) {
return failures.ErrorDifference(diff);
}
}
}

Expand Down
18 changes: 11 additions & 7 deletions lib/test-case/test-case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
SassResult,
TodoMode,
} from './util';
import {compareResults} from './compare';
import {CompareOptions, compareResults} from './compare';
import {getExpectedResult} from './expected';

/**
Expand All @@ -31,7 +31,8 @@ export default class TestCase {
dir: SpecDirectory,
impl: string,
compiler: Compiler,
todoMode: TodoMode
todoMode: TodoMode,
private compareOpts?: CompareOptions
) {
this.dir = dir;
this.impl = impl;
Expand All @@ -46,9 +47,10 @@ export default class TestCase {
dir: SpecDirectory,
impl: string,
compiler: Compiler,
todoMode?: TodoMode
todoMode?: TodoMode,
compareOpts?: CompareOptions
): Promise<TestCase> {
const testCase = new TestCase(dir, impl, compiler, todoMode);
const testCase = new TestCase(dir, impl, compiler, todoMode, compareOpts);
try {
testCase._result = await testCase.run();
} catch (caught) {
Expand Down Expand Up @@ -121,9 +123,11 @@ export default class TestCase {

const testResult = compareResults(expected, actual, {
// Compare the full error only for dart-sass
trimErrors: this.impl !== 'dart-sass',
trimErrors: this.impl !== 'dart-sass' || this.compareOpts?.trimErrors,
// Skip warning checks :warning_todo is enabled and we're not running todos
skipWarning: warningTodo && !this.todoMode,
skipWarning:
(warningTodo && !this.todoMode) || this.compareOpts?.skipWarning,
ignoreErrorDiffs: this.compareOpts?.ignoreErrorDiffs,
});
// If we're probing todo
if (this.todoMode === 'probe') {
Expand Down Expand Up @@ -185,7 +189,7 @@ export default class TestCase {
// create a warning file
if (
this.dir.hasFile('warning') &&
this.dir.readFile('warning') &&
(await this.dir.readFile('warning')) &&
actual.isSuccess &&
!actual.warning
) {
Expand Down
7 changes: 6 additions & 1 deletion sass-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ async function runAllTests() {
testDir,
args.impl,
args.compiler,
args.todoMode
args.todoMode,
{
trimErrors: args.trimErrors,
skipWarning: args.skipWarning,
ignoreErrorDiffs: args.ignoreErrorDiffs,
}
);
if (test.result().type === 'fail' && args.interactive) {
await interactor.prompt(test);
Expand Down

0 comments on commit 778aefa

Please sign in to comment.