From 2b56d7dee92718e31d3c44006866b9d6b3698dbe Mon Sep 17 00:00:00 2001 From: Kari Ikonen Date: Sat, 7 Dec 2019 22:25:51 +0200 Subject: [PATCH] FIX support either NO_COLOR or FORCE_COLOR=0 to turn off color (#3272) * FIX support either NO_COLOR or FORCE_COLOR=0 to turn off color * Deactivate colors in CLI tests, test NO_COLOR option --- cli/logging.ts | 6 + test/cli/index.js | 151 +++++++++--------- .../_config.js | 15 +- .../code-splitting-named-inputs/_config.js | 15 +- test/cli/samples/no-color/_config.js | 18 +++ test/cli/samples/no-color/main1.js | 1 + test/cli/samples/no-color/main2.js | 1 + .../samples/stdout-code-splitting/_config.js | 11 +- test/cli/samples/warn-circular/_config.js | 5 +- test/utils.js | 16 +- 10 files changed, 127 insertions(+), 112 deletions(-) create mode 100644 test/cli/samples/no-color/_config.js create mode 100644 test/cli/samples/no-color/main1.js create mode 100644 test/cli/samples/no-color/main2.js diff --git a/cli/logging.ts b/cli/logging.ts index 3c428240856..0cc1206d3cd 100644 --- a/cli/logging.ts +++ b/cli/logging.ts @@ -2,6 +2,12 @@ import tc from 'turbocolor'; import { RollupError } from '../src/rollup/types'; import relativeId from '../src/utils/relativeId'; +// @see https://no-color.org +// @see https://www.npmjs.com/package/chalk +if (process.env.FORCE_COLOR === '0' || process.env.NO_COLOR) { + tc.enabled = false; +} + // log to stderr to keep `rollup main.js > bundle.js` from breaking export const stderr = console.error.bind(console); diff --git a/test/cli/index.js b/test/cli/index.js index 13f496c171d..7b25e47623f 100644 --- a/test/cli/index.js +++ b/test/cli/index.js @@ -26,92 +26,99 @@ runTestSuiteWithSamples( const command = 'node ' + path.resolve(__dirname, '../../dist/bin') + path.sep + config.command; - const childProcess = exec(command, { timeout: 40000 }, (err, code, stderr) => { - if (err && !err.killed) { - if (config.error) { - const shouldContinue = config.error(err); + const childProcess = exec( + command, + { timeout: 40000, env: Object.assign({ FORCE_COLOR: '0' }, process.env, config.env) }, + (err, code, stderr) => { + if (err && !err.killed) { + if (config.error) { + const shouldContinue = config.error(err); + if (!shouldContinue) return done(); + } else { + throw err; + } + } + + if ('stderr' in config) { + const shouldContinue = config.stderr(stderr.trim()); if (!shouldContinue) return done(); - } else { - throw err; + } else if (stderr) { + console.error(stderr); } - } - if ('stderr' in config) { - const shouldContinue = config.stderr(stderr.trim()); - if (!shouldContinue) return done(); - } else if (stderr) { - console.error(stderr); - } + let unintendedError; - let unintendedError; + if (config.execute) { + try { + if (config.buble) { + code = buble.transform(code, { + transforms: { modules: false } + }).code; + } - if (config.execute) { - try { - if (config.buble) { - code = buble.transform(code, { - transforms: { modules: false } - }).code; - } + const fn = new Function('require', 'module', 'exports', 'assert', code); + const module = { + exports: {} + }; + fn(require, module, module.exports, assert); - const fn = new Function('require', 'module', 'exports', 'assert', code); - const module = { - exports: {} - }; - fn(require, module, module.exports, assert); + if (config.error) { + unintendedError = new Error('Expected an error while executing output'); + } - if (config.error) { - unintendedError = new Error('Expected an error while executing output'); + if (config.exports) { + config.exports(module.exports); + } + } catch (err) { + if (config.error) { + config.error(err); + } else { + unintendedError = err; + } } - if (config.exports) { - config.exports(module.exports); - } - } catch (err) { - if (config.error) { - config.error(err); - } else { - unintendedError = err; + if (config.show || unintendedError) { + console.log(code + '\n\n\n'); } - } - - if (config.show || unintendedError) { - console.log(code + '\n\n\n'); - } - if (config.solo) console.groupEnd(); + if (config.solo) console.groupEnd(); - unintendedError ? done(unintendedError) : done(); - } else if (config.result) { - try { - config.result(code); - done(); - } catch (err) { - done(err); - } - } else if (config.test) { - try { - config.test(); - done(); - } catch (err) { - done(err); - } - } else if (sander.existsSync('_expected') && sander.statSync('_expected').isDirectory()) { - try { - assertDirectoriesAreEqual('_actual', '_expected'); - done(); - } catch (err) { - done(err); - } - } else { - const expected = sander.readFileSync('_expected.js').toString(); - try { - assert.equal(normaliseOutput(code), normaliseOutput(expected)); - done(); - } catch (err) { - done(err); + unintendedError ? done(unintendedError) : done(); + } else if (config.result) { + try { + config.result(code); + done(); + } catch (err) { + done(err); + } + } else if (config.test) { + try { + config.test(); + done(); + } catch (err) { + done(err); + } + } else if ( + sander.existsSync('_expected') && + sander.statSync('_expected').isDirectory() + ) { + try { + assertDirectoriesAreEqual('_actual', '_expected'); + done(); + } catch (err) { + done(err); + } + } else { + const expected = sander.readFileSync('_expected.js').toString(); + try { + assert.equal(normaliseOutput(code), normaliseOutput(expected)); + done(); + } catch (err) { + done(err); + } } } - }); + ); childProcess.stderr.on('data', data => { if (config.abortOnStderr && config.abortOnStderr(data)) { diff --git a/test/cli/samples/code-splitting-named-default-inputs/_config.js b/test/cli/samples/code-splitting-named-default-inputs/_config.js index acc79b15858..28ac0795a3f 100644 --- a/test/cli/samples/code-splitting-named-default-inputs/_config.js +++ b/test/cli/samples/code-splitting-named-default-inputs/_config.js @@ -1,29 +1,20 @@ const assert = require('assert'); -const COLOR = '\u001b[36m\u001b[1m'; -const STANDARD = '\u001b[22m\u001b[39m'; - module.exports = { description: 'allows defining names via CLI', command: 'rollup entry1=main1.js "Entry 2"="main 2.js" "main3.js" --entryFileNames [name]-[hash].js -f es', result(code) { - let color = ''; - let standard = ''; - if (code[1] === '\u001b') { - color = COLOR; - standard = STANDARD; - } assert.equal( code, '\n' + - `${color}//→ entry1-b70571c1.js:${standard}\n` + + `//→ entry1-b70571c1.js:\n` + "console.log('main1');\n" + '\n' + - `${color}//→ Entry 2-cc781491.js:${standard}\n` + + `//→ Entry 2-cc781491.js:\n` + "console.log('main2');\n" + '\n' + - `${color}//→ main3-5e259623.js:${standard}\n` + + `//→ main3-5e259623.js:\n` + "console.log('main3');\n" ); } diff --git a/test/cli/samples/code-splitting-named-inputs/_config.js b/test/cli/samples/code-splitting-named-inputs/_config.js index ce0141c3e3d..420bde6475a 100644 --- a/test/cli/samples/code-splitting-named-inputs/_config.js +++ b/test/cli/samples/code-splitting-named-inputs/_config.js @@ -1,29 +1,20 @@ const assert = require('assert'); -const COLOR = '\u001b[36m\u001b[1m'; -const STANDARD = '\u001b[22m\u001b[39m'; - module.exports = { description: 'allows defining names via CLI', command: 'rollup --entryFileNames [name]-[hash].js --input entry1=main1.js -i "Entry 2"="main 2.js" -i "main3.js" -f es', result(code) { - let color = ''; - let standard = ''; - if (code[1] === '\u001b') { - color = COLOR; - standard = STANDARD; - } assert.equal( code, '\n' + - `${color}//→ entry1-b70571c1.js:${standard}\n` + + `//→ entry1-b70571c1.js:\n` + "console.log('main1');\n" + '\n' + - `${color}//→ Entry 2-cc781491.js:${standard}\n` + + `//→ Entry 2-cc781491.js:\n` + "console.log('main2');\n" + '\n' + - `${color}//→ main3-5e259623.js:${standard}\n` + + `//→ main3-5e259623.js:\n` + "console.log('main3');\n" ); } diff --git a/test/cli/samples/no-color/_config.js b/test/cli/samples/no-color/_config.js new file mode 100644 index 00000000000..f9796160d46 --- /dev/null +++ b/test/cli/samples/no-color/_config.js @@ -0,0 +1,18 @@ +const assert = require('assert'); + +module.exports = { + description: 'respects the NO_COLOR environment variable', + command: 'rollup -i main1.js -i main2.js -f es', + env: { FORCE_COLOR: undefined, NO_COLOR: true }, + result(code) { + assert.equal( + code, + '\n' + + '//→ main1.js:\n' + + "console.log('main1');\n" + + '\n' + + '//→ main2.js:\n' + + "console.log('main2');\n" + ); + } +}; diff --git a/test/cli/samples/no-color/main1.js b/test/cli/samples/no-color/main1.js new file mode 100644 index 00000000000..fda34828717 --- /dev/null +++ b/test/cli/samples/no-color/main1.js @@ -0,0 +1 @@ +console.log('main1'); diff --git a/test/cli/samples/no-color/main2.js b/test/cli/samples/no-color/main2.js new file mode 100644 index 00000000000..ac653633351 --- /dev/null +++ b/test/cli/samples/no-color/main2.js @@ -0,0 +1 @@ +console.log('main2'); diff --git a/test/cli/samples/stdout-code-splitting/_config.js b/test/cli/samples/stdout-code-splitting/_config.js index 65e0fa5df21..8b78d9e5424 100644 --- a/test/cli/samples/stdout-code-splitting/_config.js +++ b/test/cli/samples/stdout-code-splitting/_config.js @@ -6,20 +6,15 @@ const STANDARD = '\u001b[22m\u001b[39m'; module.exports = { description: 'bundles multiple files to stdout while adding file names', command: 'rollup -i main1.js -i main2.js -f es', + env: { FORCE_COLOR: '1' }, result(code) { - let color = ''; - let standard = ''; - if (code[1] === '\u001b') { - color = COLOR; - standard = STANDARD; - } assert.equal( code, '\n' + - `${color}//→ main1.js:${standard}\n` + + `${COLOR}//→ main1.js:${STANDARD}\n` + "console.log('main1');\n" + '\n' + - `${color}//→ main2.js:${standard}\n` + + `${COLOR}//→ main2.js:${STANDARD}\n` + "console.log('main2');" + '\n' ); diff --git a/test/cli/samples/warn-circular/_config.js b/test/cli/samples/warn-circular/_config.js index 12af2aad385..12cf9f87e77 100644 --- a/test/cli/samples/warn-circular/_config.js +++ b/test/cli/samples/warn-circular/_config.js @@ -3,6 +3,7 @@ const { assertStderrIncludes } = require('../../../utils.js'); module.exports = { description: 'warns for circular dependencies', command: 'rollup -c', - stderr: stderr => - assertStderrIncludes(stderr, '(!) Circular dependency\n' + 'main.js -> dep.js -> main.js\n') + stderr(stderr) { + assertStderrIncludes(stderr, '(!) Circular dependency\nmain.js -> dep.js -> main.js\n'); + } }; diff --git a/test/utils.js b/test/utils.js index 57791247fdb..9c0458d290f 100644 --- a/test/utils.js +++ b/test/utils.js @@ -227,10 +227,14 @@ function assertFilesAreEqual(actualFiles, expectedFiles, dirs = []) { } function assertStderrIncludes(stderr, expected) { - // eslint-disable-next-line no-control-regex - const output = stderr.replace(/\x1b\[[^m]*m/g, ''); - assert.ok( - output.includes(expected), - `Could not find ${JSON.stringify(expected)} in ${JSON.stringify(output)}` - ); + try { + assert.ok( + stderr.includes(expected), + `Could not find ${JSON.stringify(expected)} in ${JSON.stringify(stderr)}` + ); + } catch (err) { + err.actual = stderr; + err.expected = expected; + throw err; + } }