Skip to content

Commit

Permalink
fix(ci): report build script failure + fix missing logs (fabricjs#8188)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ShaMan123 @frankrousseau
ShaMan123 authored and frankrousseau committed Jan 6, 2023
1 parent ab21246 commit bd0e238
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 63 deletions.
4 changes: 1 addition & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,9 @@
"start": "node ./scripts start",
"export": "node ./scripts website export",
"build-tests": "rollup -c ./rollup.test.config.js",
"test": "node ./scripts test",
"test:unit-browser": "npm run test -- -s unit -p 8080 -l -c ",
"test:visual-browser": "npm run test -- -s visual -p 8080 -l -c ",
"test:old_but_gold": "qunit --require ./test/node_test_setup.js test/lib test/unit",
"test:visual:old_but_gold": "qunit test/node_test_setup.js test/lib test/visual",
"test": "node ./scripts test",
"test:coverage": "nyc --silent qunit test/node_test_setup.js test/lib test/unit",
"test:visual:coverage": "nyc --silent --no-clean qunit test/node_test_setup.js test/lib test/visual",
"coverage:report": "nyc report --reporter=lcov --reporter=text",
Expand Down
179 changes: 130 additions & 49 deletions scripts/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
#!/usr/bin/env node

/**
* Dearest fabric maintainer 💗,
* This file contains the cli logic, which governs most of the available commands fabric has to offer.
*
* 📢 **IMPORTANT**
* CI uses these commands.
* In order for CI to correctly report the result of the command, the process must receive a correct exit code
* meaning that if you `spawn` a process, make sure to listen to the `exit` event and terminate the main process with the relevant code.
* Failing to do so will make CI report a false positive 📉.
*/



const fs = require('fs-extra');
const os = require('os');
const _ = require('lodash');
Expand Down Expand Up @@ -121,13 +135,13 @@ inquirer.registerPrompt('test-selection', ICheckbox);


function build(options = {}) {
const args = ['rollup', '-c', options.watch ? '--watch' : ''];
const cmd = ['rollup', '-c', options.watch ? '--watch' : ''].join(' ');
let minDest;
if (options.output && !options.fast) {
const { name, base, ...rest } = path.parse(path.resolve(options.output));
minDest = path.format({ name: `${name}.min`, ...rest });
}
return cp.spawn(args.join(' '), {
const processOptions = {
stdio: 'inherit',
shell: true,
cwd: wd,
Expand All @@ -138,7 +152,21 @@ function build(options = {}) {
BUILD_OUTPUT: options.output,
BUILD_MIN_OUTPUT: minDest
},
});
}
if (options.watch) {
cp.spawn(cmd, processOptions);
}
else {
try {
cp.execSync(cmd, processOptions);
} catch (error) {
// minimal logging, no need for stack trace
console.error(error.message);
// inform ci
process.exit(1);
}
}

}

function startWebsite() {
Expand Down Expand Up @@ -244,57 +272,106 @@ function exportToWebsite(options) {
})
}


/**
*
* @param {'unit' | 'visual'} suite
* @param {string[] | null} tests file paths
* @param {{debug?:boolean,recreate?:boolean,verbose?:boolean,filter?:string}} [options]
*
* @returns {Promise<boolean | undefined>} true if some tests failed
*/
async function test(suite, tests, options = {}) {
const port = options.port || suite === 'visual' ? 8081 : 8080;
async function runTestem({ suite, port, launch, dev, processOptions, context } = {}) {
port = port || suite === 'visual' ? 8081 : 8080;
try {
await killPort(port);
} catch (error) {

}

const args = [
'testem',
!options.dev ? 'ci' : '',
if (launch) {
// open localhost
const url = `http://localhost:${port}/`;
const start = (os.platform() === 'darwin' ? 'open' : os.platform() === 'win32' ? 'start' : 'xdg-open');
cp.exec([start, url].join(' '));
}

const processCmdOptions = [
'-p', port,
'-f', `test/testem.${suite}.js`,
'-l', options.context.map(_.upperFirst).join(',')
'-l', context.map(_.upperFirst).join(',')
];

cp.spawn(args.join(' '), {
cwd: wd,
env: {
...process.env,
TEST_FILES: (tests || []).join(','),
NODE_CMD: ['qunit', 'test/node_test_setup.js', 'test/lib'].concat(tests || `test/${suite}`).join(' '),
VERBOSE: Number(options.verbose),
QUNIT_DEBUG_VISUAL_TESTS: Number(options.debug),
QUNIT_RECREATE_VISUAL_REFS: Number(options.recreate),
QUNIT_FILTER: options.filter,
REPORT_FILE: options.out
},
shell: true,
stdio: 'inherit',
detached: options.dev
})
.on('exit', (code) => {
// propagate failed exit code to the process for ci to fail
// don't exit if tests passed - this is for parallel local testing
code && process.exit(code);
if (dev) {
cp.spawn(['testem', ...processCmdOptions].join(' '), {
...processOptions,
detached: true
});
}
else {
try {
cp.execSync(['testem', 'ci', ...processCmdOptions].join(' '), processOptions);
} catch (error) {
return true;
}
}
}

if (options.launch) {
// open localhost
const url = `http://localhost:${port}/`;
const start = (os.platform() === 'darwin' ? 'open' : os.platform() === 'win32' ? 'start' : 'xdg-open');
cp.exec([start, url].join(' '));
/**
*
* @param {'unit' | 'visual'} suite
* @param {string[] | null} tests file paths
* @param {{debug?:boolean,recreate?:boolean,verbose?:boolean,filter?:string}} [options]
* @returns {Promise<boolean | undefined>} true if some tests failed
*/
async function test(suite, tests, options = {}) {
let failed = false;
const qunitEnv = {
QUNIT_DEBUG_VISUAL_TESTS: Number(options.debug),
QUNIT_RECREATE_VISUAL_REFS: Number(options.recreate),
QUNIT_FILTER: options.filter,
};
const env = {
...process.env,
TEST_FILES: (tests || []).join(','),
NODE_CMD: ['qunit', 'test/node_test_setup.js', 'test/lib'].concat(tests || `test/${suite}`).join(' '),
VERBOSE: Number(options.verbose),
REPORT_FILE: options.out
};
const browserContexts = options.context.filter(c => c !== 'node');

// temporary revert
// run node tests directly with qunit
if (options.context.includes('node')) {
try {
cp.execSync(env.NODE_CMD, {
cwd: wd,
env: {
...env,
// browser takes precendence in golden ref generation
...(browserContexts.length === 0 ? qunitEnv : {})
},
shell: true,
stdio: 'inherit',
});
} catch (error) {
failed = true;
}
}

if (browserContexts.length > 0) {
failed = await runTestem({
...options,
suite,
processOptions: {
cwd: wd,
env: {
...env,
...qunitEnv
},
shell: true,
stdio: 'inherit',
},
context: browserContexts
}) || failed;
}

return failed;
}

/**
Expand Down Expand Up @@ -410,15 +487,14 @@ async function runIntreactiveTestSuite(options) {
}
return acc;
}, { unit: [], visual: [] });
_.reduce(tests, async (queue, files, suite) => {
await queue;
return Promise.all(_.map(tests, (files, suite) => {
if (files === true) {
return test(suite, null, options);
}
else if (Array.isArray(files) && files.length > 0) {
return test(suite, files, options);
}
}, Promise.resolve());
}));
}

program
Expand Down Expand Up @@ -467,31 +543,36 @@ program
.option('-a, --all', 'run all tests', false)
.option('-d, --debug', 'debug visual tests by overriding refs (golden images) in case of visual changes', false)
.option('-r, --recreate', 'recreate visual refs (golden images)', false)
.option('-v, --verbose', 'log passing tests', false)
.option('-v, --verbose', 'log passing tests', true)
.option('--no-verbose', 'disable verbose logging')
.option('-l, --launch', 'launch tests in the browser', false)
.option('--dev', 'runs testem in `dev` mode, without a `ci` flag', false)
.addOption(new commander.Option('-c, --context <context...>', 'context to test in').choices(['node', 'chrome', 'firefox']).default(['node']))
.option('-p, --port')
.option('-o, --out <out>', 'path to report test results to')
.option('--clear-cache', 'clear CLI test cache', false)
.action((options) => {
.action(async (options) => {
if (options.clearCache) {
fs.removeSync(CLI_CACHE);
}
if (options.all) {
options.suite = ['unit', 'visual'];
}
const results = [];
if (options.suite) {
_.reduce(options.suite, async (queue, suite) => {
await queue;
results.push(...await Promise.all(_.map(options.suite, (suite) => {
return test(suite, null, options);
}, Promise.resolve());
})));
}
else if (options.file) {
test(options.file.startsWith('visual') ? 'visual' : 'unit', [`test/${options.file}`], options);
results.push(await test(options.file.startsWith('visual') ? 'visual' : 'unit', [`test/${options.file}`], options));
}
else {
runIntreactiveTestSuite(options);
results.push(...await runIntreactiveTestSuite(options));
}
if (_.some(results)) {
// inform ci that tests have failed
process.exit(1);
}
});

Expand Down
24 changes: 13 additions & 11 deletions test/node_test_setup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@ var chalk = require('chalk');
var diff = require('deep-object-diff').diff;
var commander = require('commander');

// commander.program
// .option('-d, --debug', 'debug visual tests by overriding refs (golden images) in case of visual changes', false)
// .option('-r, --recreate', 'recreate visual refs (golden images)', false)
// .action(options => {
// QUnit.debug = QUnit.debugVisual = options.debug;
// QUnit.recreateVisualRefs = options.recreate;
// }).parse(process.argv);
// // for now accept an env variable because qunit doesn't allow passing unknown options
// QUnit.debugVisual = Number(process.env.QUNIT_DEBUG_VISUAL_TESTS);
// QUnit.recreateVisualRefs = Number(process.env.QUNIT_RECREATE_VISUAL_REFS);
// QUnit.config.filter = process.env.QUNIT_FILTER;
commander.program
.option('-d, --debug', 'debug visual tests by overriding refs (golden images) in case of visual changes', false)
.option('-r, --recreate', 'recreate visual refs (golden images)', false)
.action(options => {
QUnit.debug = QUnit.debugVisual = options.debug;
QUnit.recreateVisualRefs = options.recreate;
}).parse(process.argv);
// for now accept an env variable because qunit doesn't allow passing unknown options
QUnit.debugVisual = Number(process.env.QUNIT_DEBUG_VISUAL_TESTS);
QUnit.recreateVisualRefs = Number(process.env.QUNIT_RECREATE_VISUAL_REFS);
QUnit.config.filter = process.env.QUNIT_FILTER;

process.on('uncaughtException', QUnit.onUncaughtException);

global.fabric = require('../dist/fabric').fabric;
global.pixelmatch = require('pixelmatch');
Expand Down
4 changes: 4 additions & 0 deletions test/tests.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ QUnit.config.reorder = false;
<script>
QUnit.testDone(visualCallback.testDone.bind(visualCallback));
window.addEventListener('unhandledrejection', function (event) {
QUnit.onUncaughtException(event.reason);
});
function downloadGoldens() {
const checkbox = document.getElementById('qunit-urlconfig-hidepassed');
checkbox.checked && checkbox.click();
Expand Down

0 comments on commit bd0e238

Please sign in to comment.