Skip to content

Commit

Permalink
chore: Fix tests reproducing a race condition
Browse files Browse the repository at this point in the history
  • Loading branch information
mondeja committed May 14, 2024
1 parent dca9a6b commit 59d4900
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 215 deletions.
5 changes: 0 additions & 5 deletions test/api.spec.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
import path from 'node:path';
import process from 'node:process';
import url from 'node:url';
import expect from 'expect';
import SVGLint from '../src/svglint.js';

const currentFilePath = url.fileURLToPath(import.meta.url);
const __dirname = path.dirname(currentFilePath);

process.on('unhandledRejection', (error) => {
console.error(error);
});

const svg = '<svg></svg>';

describe('.lintSource()', function () {
Expand Down
54 changes: 3 additions & 51 deletions test/attr.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import process from 'node:process';
import util from 'node:util';
import {chalk} from '../src/cli/util.js';
import SVGLint from '../src/svglint.js';

process.on('unhandledRejection', (error) => {
console.error(error);
});
import {testFailsFactory, testSucceedsFactory} from './helpers.js';

/**
* ### `attr`
Expand Down Expand Up @@ -42,49 +35,8 @@ const testSVG = `<svg role="img" viewBox="0 0 24 24">
<rect height="100" width="300" style="fill:black;" />
</svg>`;

function inspect(object) {
return chalk.reset(util.inspect(object, false, 3, true));
}

/**
* Tests that a config succeeds when ran
* @param {Config} config The config to test
* @param {String} [svg=testSVG] The SVG to lint
* @returns {Promise<void>} Throws if linting fails
*/
async function testSucceeds(config, svg = testSVG) {
const _config = {
rules: {attr: config},
};
const linting = await SVGLint.lintSource(svg, _config);
linting.on('done', () => {
if (linting.state !== linting.STATES.success) {
throw new Error(
`Linting failed (${linting.state}): ${inspect(config)}`,
);
}
});
}

/**
* Tests that a config fails when ran
* @param {Config} config The config to test
* @param {String} svg The SVG to lint
* @returns {Promise<void>} Throws if the linting doesn't fail
*/
async function testFails(config, svg = testSVG) {
const _config = {
rules: {attr: config},
};
const linting = await SVGLint.lintSource(svg, _config);
linting.on('done', () => {
if (linting.state !== linting.STATES.error) {
throw new Error(
`Linting did not fail (${linting.state}): ${inspect(_config)}`,
);
}
});
}
const testFails = testFailsFactory(testSVG, 'attr');
const testSucceeds = testSucceedsFactory(testSVG, 'attr');

describe('Rule: attr', function () {
it('should succeed without config', function () {
Expand Down
54 changes: 3 additions & 51 deletions test/custom.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import process from 'node:process';
import util from 'node:util';
import {chalk} from '../src/cli/util.js';
import SVGLint from '../src/svglint.js';

process.on('unhandledRejection', (error) => {
console.error(error);
});
import {testFailsFactory, testSucceedsFactory} from './helpers.js';

/**
* ### `custom`
Expand Down Expand Up @@ -46,49 +39,8 @@ const testSVG = `<svg role="img" viewBox="0 0 24 24">
<circle></circle>
</svg>`;

function inspect(object) {
return chalk.reset(util.inspect(object, false, 3, true));
}

/**
* Tests that a config succeeds when ran
* @param {Config} config The config to test
* @param {String} [svg=testSVG] The SVG to lint
* @returns {Promise<void>} Throws if linting fails
*/
async function testSucceeds(config, svg = testSVG) {
const _config = {
rules: {custom: config},
};
const linting = await SVGLint.lintSource(svg, _config);
linting.on('done', () => {
if (linting.state !== linting.STATES.success) {
throw new Error(
`Linting failed (${linting.state}): ${inspect(config)}`,
);
}
});
}

/**
* Tests that a config fails when ran
* @param {Config} config The config to test
* @param {String} svg The SVG to lint
* @returns {Promise<void>} Throws if the linting doesn't fail
*/
async function testFails(config, svg = testSVG) {
const _config = {
rules: {custom: config},
};
const linting = await SVGLint.lintSource(svg, _config);
linting.on('done', () => {
if (linting.state !== linting.STATES.error) {
throw new Error(
`Linting did not fail (${linting.state}): ${inspect(_config)}`,
);
}
});
}
const testFails = testFailsFactory(testSVG, 'custom');
const testSucceeds = testSucceedsFactory(testSVG, 'custom');

describe('Rule: custom', function () {
it('should succeed without config', function () {
Expand Down
51 changes: 3 additions & 48 deletions test/elm.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import process from 'node:process';
import util from 'node:util';
import {chalk} from '../src/cli/util.js';
import SVGLint from '../src/svglint.js';

process.on('unhandledRejection', (error) => {
console.error(error);
});
import {testFailsFactory, testSucceedsFactory} from './helpers.js';

/**
* ### `elm`
Expand Down Expand Up @@ -39,46 +32,8 @@ const testSVG = `<svg>
<g></g>
</svg>`;

function inspect(object) {
return chalk.reset(util.inspect(object, false, 3, true));
}

/**
* Tests that a config succeeds when ran
* @param {Config} config The config to test
* @param {String} [svg=testSVG] The SVG to lint
* @returns {Promise<void>} Throws if linting fails
*/
async function testSucceeds(config, svg = testSVG) {
const linting = await SVGLint.lintSource(svg, config);
linting.on('done', () => {
if (linting.state !== linting.STATES.success) {
throw new Error(
`Linting failed (${linting.state}): ${inspect(config)}`,
);
}
});
}

/**
* Tests that a config fails when ran
* @param {Config} config The config to test
* @param {String} svg The SVG to lint
* @returns {Promise<void>} Throws if the linting doesn't fail
*/
async function testFails(config, svg = testSVG) {
const _config = {
rules: {elm: config},
};
const linting = await SVGLint.lintSource(svg, _config);
linting.on('done', () => {
if (linting.state !== linting.STATES.error) {
throw new Error(
`Linting did not fail (${linting.state}): ${inspect(_config)}`,
);
}
});
}
const testFails = testFailsFactory(testSVG, 'elm');
const testSucceeds = testSucceedsFactory(testSVG, 'elm');

describe('Rule: elm', function () {
it('should succeed without config', function () {
Expand Down
95 changes: 95 additions & 0 deletions test/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import util from 'node:util';
import {chalk} from '../src/cli/util.js';
import SVGLint from '../src/svglint.js';

function inspect(object) {
return chalk.reset(util.inspect(object, false, 3, true));
}

export function testSucceedsFactory(svg, ruleNameOrConfig) {
/**
* Tests that a config succeeds when ran
* @param {Config} config The config to test
* @param {String} [svg=testSVG] The SVG to lint
* @returns {Promise<void>} Throws if linting fails
*/
return async (config) => {
// eslint-disable-next-line no-async-promise-executor
return new Promise(async (resolve, reject) => {
const _config =
typeof ruleNameOrConfig === 'string'
? {rules: {[ruleNameOrConfig]: config}}
: ruleNameOrConfig;
const linting = await SVGLint.lintSource(svg, _config);

// TODO: there is a race condition here. The this.lint() method
// of the Linting class is called in the constructor, so it's possible
// that the linting is already done before we call the on('done')
// event listener. Removing the next condition will make some `valid`
// rules tests fail.
if (linting.state === linting.STATES.success) {
resolve();
} else if (linting.state !== linting.STATES.linting) {
reject(
new Error(
`Linting failed (${linting.state}): ${inspect(config)}`,
),
);
}

linting.on('done', () => {
if (linting.state === linting.STATES.success) {
resolve();
} else {
reject(
new Error(
`Linting failed (${linting.state}): ${inspect(config)}`,
),
);
}
});
});
};
}

export function testFailsFactory(svg, ruleNameOrConfig) {
/**
* Tests that a config fails when ran
* @param {Config} config The config to test
* @param {String} svg The SVG to lint
* @returns {Promise<void>} Throws if the linting doesn't fail
*/
return async (config) => {
// eslint-disable-next-line no-async-promise-executor
return new Promise(async (resolve, reject) => {
const _config =
typeof ruleNameOrConfig === 'string'
? {rules: {[ruleNameOrConfig]: config}}
: ruleNameOrConfig;
const linting = await SVGLint.lintSource(svg, _config);

// TODO: Same that the TODO explained at testSucceedsFactory
if (linting.state === linting.STATES.error) {
resolve();
} else if (linting.state !== linting.STATES.linting) {
reject(
new Error(
`Linting did not fail (${linting.state}): ${inspect(_config)}`,
),
);
}

linting.on('done', () => {
if (linting.state === linting.STATES.error) {
resolve();
} else {
reject(
new Error(
`Linting did not fail (${linting.state}): ${inspect(_config)}`,
),
);
}
});
});
};
}
2 changes: 1 addition & 1 deletion test/projects/cjs/bar/.svglintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ module.exports = {
},
elm: {
"g": true,
}
},
}
};
64 changes: 5 additions & 59 deletions test/valid.spec.js
Original file line number Diff line number Diff line change
@@ -1,56 +1,14 @@
import process from 'node:process';
import util from 'node:util';
import {chalk} from '../src/cli/util.js';
import SVGLint from '../src/svglint.js';
import {testFailsFactory, testSucceedsFactory} from './helpers.js';

process.on('unhandledRejection', (error) => {
console.error(error);
});
const testFails = async (svg, config) => testFailsFactory(svg, config)();
const testSucceeds = async (svg, config) => testSucceedsFactory(svg, config)();

/**
* ### `valid`
Requires that the SVG is valid XML.
*/

function inspect(object) {
return chalk.reset(util.inspect(object, false, 3, true));
}

/**
* Tests that a config succeeds when ran
* @param {String} svg The SVG to lint
* @param {Object} [config] The config to test
* @returns {Promise<void>} Throws if linting fails
*/
async function testSucceeds(svg, config = undefined) {
const linting = await SVGLint.lintSource(svg, config);
linting.on('done', () => {
if (linting.state !== linting.STATES.success) {
throw new Error(
`Linting failed (${linting.state}),: ${inspect(config)}`,
);
}
});
}

/**
* Tests that a config fails when ran
* @param {String} svg The SVG to lint
* @param {Object} [config] The config to test
* @returns {Promise<void>} Throws if the linting doesn't fail
*/
async function testFails(svg, config = undefined) {
const linting = await SVGLint.lintSource(svg, config);
linting.on('done', () => {
if (linting.state !== linting.STATES.error) {
throw new Error(
`Linting did not fail (${linting.state}): ${inspect(config)}`,
);
}
});
}

describe('Rule: valid', function () {
it('should succeed by default for a valid SVG', function () {
return testSucceeds(
Expand Down Expand Up @@ -97,23 +55,11 @@ describe('Rule: valid', function () {
});

it('should succeed when disabled for a valid SVG', function () {
return testSucceeds(
`<svg role="img" viewBox="0 0 24 24">
<g id="foo">
<path d="bar"></path>
</g>
<g></g>
<circle></circle>
</svg>`,
{rules: {valid: false}},
);
return testSucceeds('<svg></svg>', {rules: {valid: false}});
});
it('should succeed when disabled for an invalid SVG', function () {
return testSucceeds(
`<svg viewBox="0 0 24 24" role="img">
<title>BadOne icon</title>
<path "M20.013 10.726l.001-.028A6.346"/>
</svg>`,
`<svg><path "M20.013 10.726l.001-.028A6.346"/></svg>`,
{rules: {valid: false}},
);
});
Expand Down

0 comments on commit 59d4900

Please sign in to comment.