feat(test): Miscellaneous improvements to test_tasks.js
#6615
Conversation
This file contains hidden or 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
BeksOmega
reviewed
Nov 14, 2022
| @@ -32,48 +32,84 @@ const BOLD_GREEN = '\x1b[1;32m'; | |||
| const BOLD_RED = '\x1b[1;31m'; | |||
| const ANSI_RESET = '\x1b[0m'; | |||
|
|
|||
| let failerCount = 0; | |||
| class Tester { | |||
There was a problem hiding this comment.
What's the advantage of encapsulating these in a class?
There was a problem hiding this comment.
Mainly just to remove the failerCount global variable, and allow us to also count successes without adding a second global.
Modify runTestBlock so that it can run any async task, not just ones that return a Promise, by using the async-done package (part of Gulp, and already an indirect dependency) to detect task completion. Celebrate by renaming it to runTestTask.
- Create Tester class to encapsulate the runTestTask, and reportTestResult and runAll functions. - Remove the unnecessary id parameter from runTestTask (code was already using the .name of the task function object). - Remove --silent flag from npm scripts so as not to suppress syntax error in gulpfiles.
Have the test task invoke the buildAdvancedCompilationTest (via onlyBuildAdvancedCompilationTest, to skip already-run prerequisites) directly, rather than by running npm.
b8da89a to
868cc74
Compare
|
There were merge conflicts, and it seems like I forgot a needed Just awaiting approval, assuming you think my justification for the |
BeksOmega
approved these changes
Nov 16, 2022
This was referenced Jan 11, 2023
Closed
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
Proposed Changes
runTestBlockso that it can run any async task, not just ones that return aPromise, by using theasync-donepackage (part of Gulp, and already an indirect dependency) to detect task completion. Celebrate by renaming it to runTestTask.Testerclass to encapsulate therunTestTask,reportTestResultandrunAllfunctions.--silentflag from npm scripts so as not to suppress syntax error in gulpfiles.testtask invoke thebuildAdvancedCompilationTesttask (viaonlyBuildAdvancedCompilationTest, to skip already-run prerequisites) directly, rather than by runningnpm.Reason for Changes