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

ensure bundler errors go right up the stack #808

Closed
wants to merge 5 commits into from
Closed
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
5 changes: 5 additions & 0 deletions .changeset/twenty-wombats-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@bigtest/server": patch
---

ensure bundler errors go right up the stack
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
description: 'no assertions or children'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { test } from '@bigtest/suite';

export const t = test("No default export");
40 changes: 34 additions & 6 deletions packages/server/test/manifest-builder.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe as suite, beforeEach, it } from 'mocha';
import { describe, beforeEach, it } from 'mocha';
import expect from 'expect';
import path from 'path';
import rmrf from 'rimraf';
Expand All @@ -22,7 +22,7 @@ const FIXTURES_DIR = path.resolve('test', 'fixtures');

const { mkdir, copyFile, readFile } = fs.promises;

const describe = process.platform === 'win32' ? suite.skip : suite;
// const describe = process.platform === 'win32' ? suite.skip : suite;
describe('manifest builder', () => {
let atom: Slice<OrchestratorState>;
let resultPath: string;
Expand Down Expand Up @@ -145,7 +145,7 @@ describe('manifest builder', () => {
});
});

describe('importing the manifest with an error adds the error to the state', () => {
describe('importing the manifest with a syntax error adds the error to the state', () => {
beforeEach(async () => {
await copyFile(path.join(FIXTURES_DIR, 'exceptions', 'error.t.js'), MANIFEST_PATH);
await actions.fork(atom.slice('bundler').once(({ type }) => type === 'ERRORED'));
Expand Down Expand Up @@ -177,9 +177,37 @@ describe('manifest builder', () => {
// assert is used to type narrow also and does more than just assert
assertBundlerState(bundlerState.type, {is: 'ERRORED'})

let error = bundlerState.error;
expect(bundlerState.error.message).toEqual('bork')
});
});

expect(error.message).toEqual('bork')
describe('importing a test with no default export adds the error to the state', () => {
beforeEach(async () => {
await copyFile(path.join(FIXTURES_DIR, 'exceptions', 'no-default-export.t.js'), MANIFEST_PATH);
await actions.fork(atom.slice('bundler').once(({ type }) => type === 'ERRORED'));
});
})

it('should update the global state with the error detail', () => {
let bundlerState = atom.get().bundler;

assertBundlerState(bundlerState.type, {is: 'ERRORED'})

expect(bundlerState.error.message).toContain('default export')
});
});

describe('importing an invalid test object adds the error to the state', () => {
beforeEach(async () => {
await copyFile(path.join(FIXTURES_DIR, 'exceptions', 'invalid-test-object.t.js'), MANIFEST_PATH);
await actions.fork(atom.slice('bundler').once(({ type }) => type === 'ERRORED'));
});

it('should update the global state with the error detail', () => {
let bundlerState = atom.get().bundler;

assertBundlerState(bundlerState.type, {is: 'ERRORED'})

expect(bundlerState.error.message).toContain('Test contains no assertions or children')
});
});
});
18 changes: 18 additions & 0 deletions packages/suite/src/validate-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ export class TestValidationError extends Error {
}
}

const every = Array.prototype.every;
const some = Array.prototype.some;

function validateTestKeys (test: Test, keys: (keyof Test)[], validationFn: typeof some | typeof every): boolean {
// the disable comment below is because eslint is not recognising k as used in !!test?.[k].
// eslint-disable-next-line @typescript-eslint/no-unused-vars
return validationFn.call(keys, (k: keyof Test) => !!test?.[k]);
}

function findDuplicates<T>(array: T[], callback: (value: T) => void) {
let ledger = new Set();
for(let element of array) {
Expand Down Expand Up @@ -50,6 +59,14 @@ export function validateTest(test: Test): true {
throw new TestValidationError(`Invalid Test: is too deeply nested, maximum allowed depth of nesting is ${MAXIMUM_DEPTH}\n\nTest: ${path.join(' → ')}`, file)
}

if ( validateTestKeys(test, ['description'], every) === false) {
throw new TestValidationError(`Invalid Test: Test contains no description.\n\nDoes the test file contain a default export? Test: ${path.join(' → ')}`, file);
}

if ( validateTestKeys(test, ['assertions', 'children'], some) === false) {
throw new TestValidationError(`Invalid Test: Test contains no assertions or children.\n\nTest: ${path.join(' → ')}`, test.path);
}

findDuplicates(test.assertions.map((a) => a.description), (duplicate) => {
throw new TestValidationError(`Invalid Test: contains duplicate assertion: ${JSON.stringify(duplicate)}\n\nTest: ${path.join(' → ')}`, file)
});
Expand All @@ -64,5 +81,6 @@ export function validateTest(test: Test): true {

return true;
}

return validateTestInner(test, [test.description], test.path);
}