Logging #393
Logging #393
Conversation
|
We should probably also change the issue template now that I think about it.. |
|
CI fails with a greenkeeper error. Not quite sure what's going on. |
| @@ -67,14 +67,6 @@ createIntegrationMock(); | |||
|
|
|||
| const argv = process.argv.slice(2); | |||
There was a problem hiding this comment.
All of the lines below didn't really seem to.. do anything. So I removed them.
| @@ -7,7 +7,7 @@ | |||
| "scripts": { | |||
| "build": "cpx index.d.ts dist/ && tsc -p .", | |||
| "build:watch": "cpx index.d.ts dist/ && tsc -p . -w", | |||
| "clean": "rimraf dist/**/* && rimraf tests/simple/coverage && rimraf tests/simple-async/coverage", | |||
| "clean": "rimraf dist/**/* && rimraf tests/simple/coverage && rimraf tests/simple-async/coverage && rimraf tests/**/*/debug.txt", | |||
| "clean-build": "npm run clean && npm run build", | |||
There was a problem hiding this comment.
it might be better to create a separate directory for debug logs than to have them scattered all over and then delete them like this.
There was a problem hiding this comment.
Yeah I considered just adding the debug.txt in the project root, but we'd still need to delete them for each subproject yeah?
There was a problem hiding this comment.
Sorry about this. I didn't notice that the delete was recursive under the tests directory. This should be fine. I was somehow under the impression that this will be done throughout the project.
I now have a different question. These logs will be produced when debugging sample repos, right? If so, will the logs still be placed under ts-jest in node_modules?
There was a problem hiding this comment.
Yep. That's what I figured made most sense.
There was a problem hiding this comment.
I'm not entirely sure about this so lets just do it this way and then, if required, revisit it
src/logger.ts
Outdated
| config.globals && | ||
| config.globals['ts-jest'] && | ||
| config.globals['ts-jest'].debug | ||
| ) { |
There was a problem hiding this comment.
using an environment variable might be a better option (process.env.TS_JEST_DEBUG, for example)
src/preprocessor.ts
Outdated
| @@ -12,21 +13,29 @@ export function process( | |||
| config: JestConfig, | |||
| transformOptions: TransformOptions = { instrument: false }, | |||
| ) { | |||
| enableLoggingIfNeeded(config); | |||
There was a problem hiding this comment.
this line moves the comment below which makes the comment lose some context
There was a problem hiding this comment.
Nicely spotted. I'll move it.
|
Do you know what's up with the greenkeeper bug? |
I'm not sure what's happening but maybe we should just comment that part out in the travis config for now
not a clue
alright. Would you have the time to merge this into that branch? It shouldn't be a problem but there could be some merge conflicts, I'm guessing |
|
@GeeWee jest 22 has been published and 'cleanup' has been merged :) |
|
@GeeWee is this still on the cards? |
|
Yeah looking at it today. Been on vacation! |
Conflicts: src/preprocessor.ts src/utils.ts
|
I think I'm good for a new review @kulshekhar |
|
do we need a version bump in this or can that come later? |
|
Awesome. I think we might want to, because this updates the issue template, so we'll risk people trying the new instructions without them being available. |
|
yeah, just bump the version patch in package.json. I'll then approve, merge and publish the package to npm |
|
bumped the version number! |
|
published on npm! This will be quite useful @GeeWee 👍 |
I've done a few things here.
I've added logging as per #388 . Feel free to comment on the logging messages, didn't spend a lot of time considering those.
I've also done a little bit to our testing, the place was littered with toString() calls on strings, and I had some problems with string matching due to ANSI color codes getting in the way.
So I've cleaned up the toString stuff and I've removed the ANSI color codes when running in tests.
I've also tweaked some of the more rigorous tslint rules that prettier fixed anyways, and ran prettier on the whole project. Seems it's shifted things a little bit around, not quite sure why. Perhaps some commits got past the precommit hook somehow.