From a7f498a6a02ae795aad1dd6f4f3dfb74a703ffa4 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 26 Jan 2023 20:09:19 +0000 Subject: [PATCH] fix(build): Fix event tests, improve `buildDeps` (#6773) * fix(tests): Fix errors in event tests - Fix actual syntax errors in imports in event_marker_move_test.js and event_selected.test.js, which were preventing those tests from being run. - Remove suite.only directives in those tests that would prevent all the other tests from running. * refactor(build): Improve buildDeps - Run closure-make-deps only once, instead of separately for core/ and tests/. - Specify a larger exec maxBuffer size, to ensure output and diagnostics are not truncated. - Change stderr filtering in buildDeps to filter out bounded generics messages and blank lines. - Attempt to suppress warnings in stderr output when closure-make-deps returns a non-zero exit code. Unfortunately, there seems to be a race condition which usually the stderr argument to the exec callback not to contain the complete output, so in that case print a helpful message. - Have buildDeps just return a Promise instead of using a callback. * fix(docs): Typo fix in JSDoc for log helper --- scripts/gulpfiles/build_tasks.js | 87 +++++++++++++++------------ tests/mocha/event_marker_move_test.js | 4 +- tests/mocha/event_selected_test.js | 4 +- 3 files changed, 51 insertions(+), 44 deletions(-) diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index 9aaa7dc1d..eacbc2c88 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -309,71 +309,78 @@ function buildJavaScript(done) { * * Prerequisite: buildJavaScript. */ -function buildDeps(done) { +function buildDeps() { const roots = [ path.join(TSC_OUTPUT_DIR, 'closure', 'goog', 'base.js'), TSC_OUTPUT_DIR, 'blocks', 'generators', + 'tests/mocha', ]; - const testRoots = [ - ...roots, - 'tests/mocha' - ]; + /** Maximum buffer size, in bytes for child process stdout/stderr. */ + const MAX_BUFFER_SIZE = 10 * 1024 * 1024; /** - * Extracts lines that contain the specified keyword. - * @param {string} text output text - * @param {string} keyword extract lines with this keyword - * @returns {string} modified text + * Filter a string to extract lines containing (or not containing) the + * specified target string. + * + * @param {string} text Text to filter. + * @param {string} target String to search for. + * @param {boolean?} exclude If true, extract only non-matching lines. + * @returns {string} Filtered text. */ - function extractOutputs(text, keyword) { + function filter(text, target, exclude) { return text.split('\n') - .filter((line) => line.includes(keyword)) + .filter((line) => Boolean(line.match(target)) !== Boolean(exclude)) .join('\n'); } - function filterErrors(text) { - return text.split('\n') - .filter( - (line) => !/^WARNING /.test(line) || - !(/Missing type declaration./.test(line) || - /illegal use of unknown JSDoc tag/.test(line))) - .join('\n'); + /** + * Log unexpected diagnostics, after removing expected warnings. + * + * @param {string} text Standard error output from closure-make-deps + */ + function log(text) { + for (const line of text.split('\n')) { + if (line && + !/^WARNING .*: Bounded generic semantics are currently/.test(line) && + !/^WARNING .*: Missing type declaration/.test(line) && + !/^WARNING .*: illegal use of unknown JSDoc tag/.test(line)) { + console.error(line); + } + } } - new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const args = roots.map(root => `--root '${root}' `).join(''); exec( - `closure-make-deps ${args}`, + `closure-make-deps ${args}`, {maxBuffer: MAX_BUFFER_SIZE}, (error, stdout, stderr) => { - console.warn(filterErrors(stderr)); if (error) { + // Remove warnings from stack trace to show only errors. + error.stack = filter(error.stack, /^WARNING/, true); + // Due to some race condition, the stderr parameter is + // often badly truncated if an error is non-null, so the + // error message might not actually be shown to the user. + // Print a helpful message to the user to help them find + // out what the problem is. + error.stack += ` + +If you do not see an helpful diagnostic from closure-make-deps in the +error message above, try running: + + npx closure-make-deps ${args} 2>&1 |grep -v WARNING`; reject(error); } else { - fs.writeFileSync(DEPS_FILE, stdout); + log(stderr); + // Anything not about mocha goes in DEPS_FILE. + fs.writeFileSync(DEPS_FILE, filter(stdout, 'tests/mocha', true)); + // Anything about mocha does in TEST_DEPS_FILE. + fs.writeFileSync(TEST_DEPS_FILE, filter(stdout, 'tests/mocha')); resolve(); } }); - }).then(() => new Promise((resolve, reject) => { - // Filter out the entries that are already in deps.js. - const testArgs = - testRoots.map(root => `--root '${root}' `).join(''); - exec( - `closure-make-deps ${testArgs}`, - (error, stdout, stderr) => { - console.warn(filterErrors(stderr)); - if (error) { - reject(error); - } else { - fs.writeFileSync(TEST_DEPS_FILE, - extractOutputs(stdout, 'tests/mocha')); - resolve(); - } - }); - })).then(() => { - done(); }); } diff --git a/tests/mocha/event_marker_move_test.js b/tests/mocha/event_marker_move_test.js index e7ddac2f8..f44ca330a 100644 --- a/tests/mocha/event_marker_move_test.js +++ b/tests/mocha/event_marker_move_test.js @@ -7,7 +7,7 @@ goog.declareModuleId('Blockly.test.eventMarkerMove'); -import {defineRowBlock} from './test_helpers/block_definitions.js;'; +import {defineRowBlock} from './test_helpers/block_definitions.js'; import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown.js'; @@ -22,7 +22,7 @@ suite('Marker Move Event', function() { sharedTestTeardown.call(this); }); - suite.only('Serialization', function() { + suite('Serialization', function() { test('events round-trip through JSON', function() { const block1 = this.workspace.newBlock('row_block', 'test_id1'); const block2 = this.workspace.newBlock('row_block', 'test_id2'); diff --git a/tests/mocha/event_selected_test.js b/tests/mocha/event_selected_test.js index 726aba93a..f7be3745d 100644 --- a/tests/mocha/event_selected_test.js +++ b/tests/mocha/event_selected_test.js @@ -7,7 +7,7 @@ goog.declareModuleId('Blockly.test.eventSelected'); -import {defineRowBlock} from './test_helpers/block_definitions.js;'; +import {defineRowBlock} from './test_helpers/block_definitions.js'; import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown.js'; @@ -22,7 +22,7 @@ suite('Selected Event', function() { sharedTestTeardown.call(this); }); - suite.only('Serialization', function() { + suite('Serialization', function() { test('events round-trip through JSON', function() { const block1 = this.workspace.newBlock('row_block', 'test_id1'); const block2 = this.workspace.newBlock('row_block', 'test_id2');