Skip to content

Commit

Permalink
chore: Fix tests (reproduces a race condition bug) (#101)
Browse files Browse the repository at this point in the history
  • Loading branch information
mondeja authored May 14, 2024
1 parent 83c8e4e commit 3ce8e62
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 218 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
64 changes: 9 additions & 55 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 All @@ -97,13 +49,15 @@ describe('Rule: custom', function () {

it('should provide file information', function () {
return testSucceeds([
(_reporter, _$, _ast, info) => {
(reporter, _$, _ast, info) => {
if (!info) {
throw new Error('no info provided');
reporter.error('no info provided');
}

if (!Object.hasOwn(info, 'filepath')) {
throw new Error('no filepath provided on info');
// NOTE: Object.hasOwn is not available at Node.js v12 nor v14
// eslint-disable-next-line prefer-object-has-own
if (!Object.prototype.hasOwnProperty.call(info, 'filepath')) {
reporter.error('no filepath provided on info');
}
},
]);
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)}`,
),
);
}
});
});
};
}
Loading

0 comments on commit 3ce8e62

Please sign in to comment.