fix(build): Fix event tests, improve buildDeps
#6773
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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.
- 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.
NeilFraser
reviewed
Jan 20, 2023
scripts/gulpfiles/build_tasks.js
Outdated
| /** | ||
| * Log unexpected diagnostics, after removing expected warnings. | ||
| * | ||
| * @param {string} test Standard error output from closure-make-deps |
NeilFraser
approved these changes
Jan 20, 2023
maribethb
changed the title
buildDepsbuildDeps
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The basics
npm run formatandnpm run lintThe details
Resolves
npm testoutput had become increasingly cluttered with miscellaneous WARNINGs fromclosure-make-deps. Investigating this, it turned out that two recently-added test files were not being run at all.Proposed Changes
Fix actual syntax errors in
imports inevent_marker_move_test.jsandevent_selected.test.js, which were preventing those tests from being run.Remove
suite.onlydirectives in those tests that would prevent all the other tests from running.Run
closure-make-depsonly once, instead of separately forcore/andtests/.Specify a larger
execmaxBuffersize, to ensure output and diagnostics are not truncated.Change
stderrfiltering inbuildDepsto filter out bounded generics messages and blank lines.Attempt to suppress warnings in
stderroutput whenclosure-make-depsreturns a non-zero exit code.Unfortunately, there seems to be a race condition which usually causes the
stderrargument to the exec callback not to contain the complete output, so in that case print a helpful message.Have
buildDepsjust return aPromiseinstead of using a callback.Behaviour Before Change
Two of the tests were not being run.
Behaviour After Change
All tests are run;
buildDepsruns about twice as fast as before.Test Coverage
Improved.