-
-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: gather process tree information #364
Conversation
@addaleax I like this as a start to addressing #158! It looks like the tests are failing because Travis is outputting the hierarchy in a different ordering than your local machine, it outputs branch A couple thoughts:
|
Done! I definitely should have thought of that… 😄
Sounds like a good idea… I’ve started splitting things up, I guess I’ll see if I get the self-coverage handling right.
Definitely, and I agree that that would a pretty neat thing to have! It might require touching a bit more of the reporting logic, though. |
@@ -326,7 +336,7 @@ NYC.prototype.clearCache = function () { | |||
} | |||
|
|||
NYC.prototype.createTempDirectory = function () { | |||
mkdirp.sync(this.tempDirectory()) | |||
mkdirp.sync(this.processInfoDirectory()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we make it so we only create this directory, if we've enabled process tracking; my thinking is, let's make the behavior 100% identical, unless you are outputting the process tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, I’ll try that
Let's hold off on this until V2 💃 I made a few more comments, also we should definitely figure out a good place to document this in the README; this having been said, I'm really loving this feature. |
You mean, this PR, or per-subtree coverage reporting? I’m fine with either, but if you’re talking about the latter, I’d want to try and implement per-subtree reporting to make sure everything’s wired up in order for that to work (edit: … i took the liberty to throw away all old coveralls comments which were clobbering up everything here) |
Add a `--show-process-tree` that shows a pretty tree of all spawned processes after `nyc` has run. The data files for that are stored in `processinfo` are stored in `(temp directory)/processinfo` so that they don’t interfere with the fixed format of the coverage files. Fixes: istanbuljs#158
…lves This is in preparation for per-subtree coverage at some point.
Changes Unknown when pulling 88021d0 on addaleax:process-tree into * on istanbuljs:master*. |
@addaleax I think this pull request is great, and we should land it ASAP -- just want to get the behavior identical if it's not enabled, and add some documentation to the README 👍 |
I think that’s the case. :)
Done! Please make sure this PR is in the state you’d like it to see in :) |
Changes Unknown when pulling 3dad767 on addaleax:process-tree into * on istanbuljs:master*. |
@addaleax awesome, on a short vacation to NYC, but will hopefully land some pull requests tomorrow or Monday. |
That's right, @bcoe is so committed to Open Source, he takes vacations just to visit his node modules. |
@isaacs … glad to hear your dad joke license arrived ;) (ok truth is i have been giggling at this for five minutes straight) |
@addaleax mind giving the
if QA is looking good, we'll get this promoted soon. |
@bcoe Everything looks good so far :) |
Add a
--show-process-tree
that shows a pretty tree of all spawned processes afternyc
has run.The data files for that are stored in
processinfo
are stored in(temp directory)/processinfo
so that they don’t interfere with the fixed format of the coverage files.Fixes: #158
Right now this only displays the commands as they are being run, ex:
Note: Unlike described in the above issue, I’ve opted to leave environment variables out of the stored data because they may contain sensitive information and there have been cases of
.nyc_output
accidentally being published.Any thoughts?