Skip to content

Commit

Permalink
Get rid of "--delay" mocha CLI option (#364)
Browse files Browse the repository at this point in the history
This is needed to fix #363. The `--delay` option requires the code to call the
`run` function only once and we were calling it multiple times. More
importantly, Mocha does not catch exceptions thrown at the root level and simply
reports nothing when that happens. These exceptions typically get thrown when
the IDL cannot be parsed, meaning that invalid data exceptions mostly went
unnoticed...

This update wraps all code that needs to run before tests in a `before`
function. Note the need to keep a "dummy" test at the root level, otherwise the
`before` function won't run (and wouldn't have anywhere to report failure).

This update slightly improves describe/it tests naming as well.
  • Loading branch information
tidoust committed Sep 22, 2021
1 parent 833f1a7 commit b3600e7
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 56 deletions.
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
"scripts": {
"create-patch": "node tools/create-patch.js",
"prepare": "node packages/prepare.js",
"test": "mocha --recursive --delay",
"test-css": "mocha --recursive --delay test/css",
"test-elements": "mocha --recursive --delay test/elements",
"test-idl": "mocha --recursive --delay test/idl"
"test": "mocha --recursive",
"test-css": "mocha --recursive test/css",
"test-elements": "mocha --recursive test/elements",
"test-idl": "mocha --recursive test/idl"
}
}
4 changes: 2 additions & 2 deletions test/css/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const assert = require('assert').strict;

const css = require('@webref/css');

describe('@webidl/css module', () => {
it('listAll', async () => {
describe('The @webref/css module', () => {
it('can list all CSS properties (listAll)', async () => {
const all = await css.listAll();
assert(Object.keys(all).length > 0);
for (const desc of Object.values(all)) {
Expand Down
4 changes: 2 additions & 2 deletions test/css/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const assert = require('assert').strict;
const cssPackage = require('../../packages/css/package.json');
const rootPackage = require('../../package.json');

describe('@webidl/css package', () => {
it('uses same version as main package', () => {
describe('The @webref/css package', () => {
it('uses the same version of css-tree as main package', () => {
assert.equal(cssPackage.peerDependencies['css-tree'],
`^${rootPackage.devDependencies['css-tree']}`);
});
Expand Down
50 changes: 29 additions & 21 deletions test/css/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,34 @@ const tempIgnore = [
{ shortname: 'svg-strokes', prop: 'valuespaces', name: '<dasharray>' }
];

css.listAll().then(all => {
for (const [shortname, data] of Object.entries(all)) {
describe(`The ${shortname} entry in @webidl/css`, () => {
for (const { type, prop, value } of cssValues) {
for (const [name, desc] of Object.entries(data[prop])) {
if (!desc[value]) {
continue;
}
if (tempIgnore.some(c => c.shortname === shortname &&
c.prop === prop && c.name === name)) {
continue;
describe('The @webref/css module entries', () => {
before(async () => {
const all = await css.listAll();

for (const [shortname, data] of Object.entries(all)) {
describe(`The ${shortname} entry in @webref/css`, () => {
for (const { type, prop, value } of cssValues) {
for (const [name, desc] of Object.entries(data[prop])) {
if (!desc[value]) {
continue;
}
if (tempIgnore.some(c => c.shortname === shortname &&
c.prop === prop && c.name === name)) {
continue;
}
it(`defines a valid ${type} "${name}"`, () => {
assert.doesNotThrow(() => {
const ast = definitionSyntax.parse(desc[value]);
assert(ast.type);
}, `Invalid definition value: ${desc[value]}`);
});
}
it(`defines a valid ${type} "${name}"`, () => {
assert.doesNotThrow(() => {
const ast = definitionSyntax.parse(desc[value]);
assert(ast.type);
}, `Invalid definition value: ${desc[value]}`);
});
}
}
});
}
}).then(run);
});
}
});

// Dummy test needed for "before" to run and register late tests
// (test will fail if before function throws, e.g. because data is invalid)
it('can initialize data', () => {});
});
4 changes: 2 additions & 2 deletions test/elements/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const assert = require('assert').strict;

const elements = require('@webref/elements');

describe('@webidl/elements module', () => {
it('listAll', async () => {
describe('The @webref/elements module', () => {
it('can list all elements', async () => {
const all = await elements.listAll();
assert(Object.keys(all).length > 0);
for (const desc of Object.values(all)) {
Expand Down
29 changes: 18 additions & 11 deletions test/elements/consistency.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@ const assert = require('assert').strict;
const elements = require('@webref/elements');
const idl = require('@webref/idl');

idl.parseAll().then(allIdl => {
// Create a set of well-known interfaces
const interfaces = new Set();
for (const [shortname, ast] of Object.entries(allIdl)) {
for (const dfn of ast) {
if (dfn.name) {
interfaces.add(dfn.name);
describe('@webref/elements module entries', () => {
before(async () => {
const allIdl = await idl.parseAll();

// Create a set of well-known interfaces
const interfaces = new Set();
for (const [shortname, ast] of Object.entries(allIdl)) {
for (const dfn of ast) {
if (dfn.name) {
interfaces.add(dfn.name);
}
}
}
}

return elements.listAll().then(allElements => {
const allElements = await elements.listAll();
for (const [shortname, data] of Object.entries(allElements)) {
describe(`The ${shortname} entry in @webidl/elements`, () => {
describe(`The ${shortname} entry in @webref/elements`, () => {
for (const el of data.elements) {
if (!el.interface) {
continue;
Expand All @@ -28,4 +31,8 @@ idl.parseAll().then(allIdl => {
});
}
});
}).then(run);

// Dummy test needed for "before" to run and register late tests
// (test will fail if before function throws, e.g. because data is invalid)
it('can initialize data', () => {});
});
6 changes: 3 additions & 3 deletions test/idl/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const assert = require('assert').strict;

const idl = require('@webref/idl');

describe('@webidl/idl module', () => {
it('listAll', async () => {
describe('The @webref/idl module', () => {
it('can list all IDL (listAll)', async () => {
const files = await idl.listAll();
assert(Object.keys(files).length > 0);
for (const [shortname, file] of Object.entries(files)) {
Expand All @@ -13,7 +13,7 @@ describe('@webidl/idl module', () => {
}
});

it('parseAll', async () => {
it('can parse all IDL (parseAll)', async () => {
const all = await idl.parseAll();
assert(Object.keys(all).length > 0);
for (const ast of Object.values(all)) {
Expand Down
4 changes: 2 additions & 2 deletions test/idl/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const assert = require('assert').strict;
const idlPackage = require('../../packages/idl/package.json');
const rootPackage = require('../../package.json');

describe('@webidl/idl package', () => {
it('webidl2.js dependency version', () => {
describe('The @webref/idl package', () => {
it('uses the same version of webidl2.js as main package', () => {
assert.equal(idlPackage.peerDependencies.webidl2,
`^${rootPackage.devDependencies.webidl2}`);
});
Expand Down
29 changes: 20 additions & 9 deletions test/idl/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,29 @@ function validate(ast) {
assert.fail(message);
}

idl.parseAll().then((all) => {
describe('WebIDL2.validate', () => {
for (const [spec, ast] of Object.entries(all)) {
it(spec, () => {
validate(ast);
});
}
describe('The @webref/idl data', async () => {
before(async () => {
const all = await idl.parseAll();

// Register late tests
// (note "describe" level is needed, and there needs to remain an "it" at
// the parent "describe" level)
describe('The @webref/idl data is valid per spec', () => {
for (const [spec, ast] of Object.entries(all)) {
it(spec, () => {
validate(ast);
});
}
});

// Also validate all IDL together. Some validation rules have insufficient
// type information when looking at each spec in isolation.
it('all IDL together', () => {
it('is valid when IDL from all specs gets combined', () => {
validate(Object.values(all).flat());
});
});
}).then(run);

// Dummy test needed for "before" to run and register late tests
// (test will fail if before function throws, e.g. because data is invalid)
it('can initialize data', () => {});
});

0 comments on commit b3600e7

Please sign in to comment.