Skip to content

Commit

Permalink
Detect t.try() usage in hooks; experimentally disable t.snapshot()
Browse files Browse the repository at this point in the history
`t.try()` has never worked in hooks. Fail the hook properly instead of letting it crash.

`t.snapshot()` sort of works, but not really. Add an experiment so support can be disabled. This will become the default behavior in the next major AVA release.

Fixes #2523.

Co-authored-by: Mark Wubben <mark@novemberborn.net>
  • Loading branch information
oantoro and novemberborn committed Jul 4, 2020
1 parent eee2f7b commit d01db61
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 3 deletions.
12 changes: 11 additions & 1 deletion lib/assert.js
Expand Up @@ -258,7 +258,8 @@ class Assertions {
skip = notImplemented,
compareWithSnapshot = notImplemented,
powerAssert,
experiments = {}
experiments = {},
disableSnapshots = false
} = {}) {
const withSkip = assertionFn => {
assertionFn.skip = skip;
Expand Down Expand Up @@ -699,6 +700,15 @@ class Assertions {
});

this.snapshot = withSkip((expected, ...rest) => {
if (disableSnapshots && experiments.disableSnapshotsInHooks) {
fail(new AssertionError({
assertion: 'snapshot',
message: '`t.snapshot()` can only be used in tests',
improperUsage: true
}));
return;
}

let message;
let snapshotOptions;
if (rest.length > 1) {
Expand Down
2 changes: 1 addition & 1 deletion lib/load-config.js
Expand Up @@ -7,7 +7,7 @@ const pkgConf = require('pkg-conf');

const NO_SUCH_FILE = Symbol('no ava.config.js file');
const MISSING_DEFAULT_EXPORT = Symbol('missing default export');
const EXPERIMENTS = new Set(['likeAssertion', 'reverseTeardowns']);
const EXPERIMENTS = new Set(['disableSnapshotsInHooks', 'likeAssertion', 'reverseTeardowns']);

// *Very* rudimentary support for loading ava.config.js files containing an `export default` statement.
const evaluateJsConfig = configFile => {
Expand Down
9 changes: 8 additions & 1 deletion lib/test.js
Expand Up @@ -40,7 +40,8 @@ class ExecutionContext extends assert.Assertions {
return test.compareWithSnapshot(options);
},
powerAssert: test.powerAssert,
experiments: test.experiments
experiments: test.experiments,
disableSnapshots: test.isHook === true
});
testMap.set(this, test);

Expand Down Expand Up @@ -74,6 +75,12 @@ class ExecutionContext extends assert.Assertions {
};

this.try = async (...attemptArgs) => {
if (test.isHook) {
const error = new Error('`t.try()` can only be used in tests');
test.saveFirstError(error);
throw error;
}

const {args, buildTitle, implementations, receivedImplementationArray} = parseTestArgs(attemptArgs);

if (implementations.length === 0) {
Expand Down
10 changes: 10 additions & 0 deletions test/helpers/exec.js
Expand Up @@ -44,6 +44,7 @@ exports.fixture = async (...args) => {
const errors = new WeakMap();
const stats = {
failed: [],
failedHooks: [],
passed: [],
skipped: [],
uncaughtExceptions: [],
Expand All @@ -59,6 +60,14 @@ exports.fixture = async (...args) => {
}

switch (statusEvent.type) {
case 'hook-failed': {
const {title, testFile} = statusEvent;
const statObject = {title, file: normalizePath(cwd, testFile)};
errors.set(statObject, statusEvent.err);
stats.failedHooks.push(statObject);
break;
}

case 'selected-test': {
if (statusEvent.skip) {
const {title, testFile} = statusEvent;
Expand Down Expand Up @@ -108,6 +117,7 @@ exports.fixture = async (...args) => {
throw Object.assign(error, {stats});
} finally {
stats.failed.sort(compareStatObjects);
stats.failedHooks.sort(compareStatObjects);
stats.passed.sort(compareStatObjects);
stats.skipped.sort(compareStatObjects);
stats.unsavedSnapshots.sort(compareStatObjects);
Expand Down
9 changes: 9 additions & 0 deletions test/hook-restrictions/fixtures/invalid-snapshots-in-hooks.js
@@ -0,0 +1,9 @@
const test = require('ava');

test.before(t => {
t.snapshot({});
});

test('cannot use snapshot in hook', t => {
t.pass();
});
9 changes: 9 additions & 0 deletions test/hook-restrictions/fixtures/invalid-t-try-in-hooks.js
@@ -0,0 +1,9 @@
const test = require('ava');

test.before(async t => {
await t.try(tt => tt.pass());
});

test('cannot use `t.try()` in hook', t => {
t.pass();
});
10 changes: 10 additions & 0 deletions test/hook-restrictions/fixtures/package.json
@@ -0,0 +1,10 @@
{
"ava": {
"files": [
"*.js"
],
"nonSemVerExperiments": {
"disableSnapshotsInHooks": true
}
}
}
17 changes: 17 additions & 0 deletions test/hook-restrictions/snapshots/test.js.md
@@ -0,0 +1,17 @@
# Snapshot report for `test/hook-restrictions/test.js`

The actual snapshot is saved in `test.js.snap`.

Generated by [AVA](https://avajs.dev).

## `t.try()` cannot be used in hooks

> error message
'`t.try()` can only be used in tests'

## snapshots cannot be used in hooks

> error message
'`t.snapshot()` can only be used in tests'
Binary file added test/hook-restrictions/snapshots/test.js.snap
Binary file not shown.
14 changes: 14 additions & 0 deletions test/hook-restrictions/test.js
@@ -0,0 +1,14 @@
const test = require('@ava/test');
const exec = require('../helpers/exec');

test('snapshots cannot be used in hooks', async t => {
const result = await t.throwsAsync(exec.fixture('invalid-snapshots-in-hooks.js'));
const error = result.stats.getError(result.stats.failedHooks[0]);
t.snapshot(error.message, 'error message');
});

test('`t.try()` cannot be used in hooks', async t => {
const result = await t.throwsAsync(exec.fixture('invalid-t-try-in-hooks.js'));
const error = result.stats.getError(result.stats.failedHooks[0]);
t.snapshot(error.message, 'error message');
});

0 comments on commit d01db61

Please sign in to comment.