Add low-level print configuration option. #130
Conversation
|
Three tests do not pass, but let me see if you are up for this kind of change before I invest more time to fix them. |
|
It is a great option, go for it! |
|
Ready for review. |
Codecov Report
@@ Coverage Diff @@
## master #130 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 14 14
Lines 411 413 +2
Branches 54 54
=====================================
+ Hits 411 413 +2
Continue to review full report at Codecov.
|
src/configuration-parser.ts
Outdated
| @@ -19,6 +19,8 @@ export class ConfigurationParser { | |||
| pending: "* ", | |||
| successful: ConfigurationParser.isWindows ? "\u221A " : "✓ ", | |||
| }, | |||
| // tslint:disable-next-line:no-unbound-method | |||
| print: console.log, | |||
There was a problem hiding this comment.
Any reason to reference directly console.log?
By using a wrapper we could spare us the console.log mock
There was a problem hiding this comment.
TestHelper needs to mock the default print feature so the (non-custom-print) tests can examine the reporter output. ConfigurationParser needs a default print feature. So we need some place where TestHelper and ConfigurationParser can agree to use for the default print feature. They current agree on the global "console.log".
We could use another global, or we could add a static to any class. TestHelper needs to access that wrapper to overwrite it for mocking.
I took a stab, is this what you have in mind?
The other way to go is to have TestHelper overwrite this.reporter.print
There was a problem hiding this comment.
My point is that console.log was already instrumented for non-custom-print tests, so I wondered why do we need to also mock it.
My understanding is that we need to do that because we are referencing directly the console.log function in:
print: console.log
Then, we need to instrument the console.log function before the execution of configuration-parser.ts.
If we use a console.log wrapper for the print function:
print: stuff => console.log(stuff)
Then, we would not have the need to early mock the log function and the previous instrumentation should be enough.
Am I missing something?
Defaults to console.log. Set to function process.stdout.write(log + '\n'); to avoid output to devtools console while still reporting jasmine results to command line.
|
Available in [email protected] |
Defaults to console.log. Set to function process.stdout.write(log + '\n'); to avoid output to
devtools console while still reporting jasmine results to command line.