Skip to content
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

Way to tag executed lines with test identifier? #597

Closed
wysisoft opened this issue Jun 7, 2017 · 10 comments
Closed

Way to tag executed lines with test identifier? #597

wysisoft opened this issue Jun 7, 2017 · 10 comments

Comments

@wysisoft
Copy link

wysisoft commented Jun 7, 2017

It would be really awesome if I could tell nyc/istanbul which test is currently running, and it could tag the lines of code executed with the most recent test identifier, so I know which tests are covering which code areas. Any way to make this happen?

@wysisoft
Copy link
Author

wysisoft commented Jun 9, 2017

Also, if this gets implemented, I hope that it works with instrument, since that's what I use - instrumented .js files and selenium

@bcoe
Copy link
Member

bcoe commented Jul 29, 2017

@wysisoft not quite sure how we'd pull this off in an elegant way; would definitely like to pull it into a module that does all the bookkeeping, and is enabled optionally, but I really like the idea!

It's kind of similar to the work @addaleax did on tracking subprocess information.

@bcoe
Copy link
Member

bcoe commented Jul 29, 2017

would love help implementing if anyone is interested.

@bcoe bcoe added the triaged label Jul 29, 2017
@wysisoft
Copy link
Author

wysisoft commented Jul 29, 2017

I might be over simplifying it, but you already have a global variable (I forget its name, something like __ globalVar __), I thought you can just add a property called something like testName and whatever the current value of __ globalVar __.testName is recorded along with the rest of the instrumentation information, and then its up to users like me to propertly set the __ globalVar __.testName value at the start of each individual test? Is this too simple?

@bcoe
Copy link
Member

bcoe commented Jul 29, 2017

@wysisoft sounds like this could work reasonably well, the global variable being set as tests run is __coverage__ which has coverage information for each file; I would definitely want to only enable this with a flag, as it has the potential to slow down the overall test run.

@wysisoft
Copy link
Author

Ya I might be alone on this but I think that actually knowing which test provides which coverage is critical for maintaining my code base, also for getting a good place to start when coverage is missing.

@wysisoft
Copy link
Author

Keep in mind that i'm the worst developer in the world.

So i'd propose having another global object called coverageTests.
In instanbul's instrumenter.js, I think nyc needs to override the AST 'cover' functions so it does another line, something like coverageTests.s[id] = (coverageTests.s[id].indexOf(coverage.currentTest + ';') == -1 ? coverageTests.s[id] + ';' : coverageTests.s[id]);

then in addDerivedInfoForFile you add a new variable on the coverage, coverage.tl (lineTests), and populate the tests for the line.

Then in the html.js detail template you add a td after the linenumber called lineTests and populate it with the values from coverage.tl

@bcoe
Copy link
Member

bcoe commented Aug 1, 2017

@wysisoft I haven't thought too hard about this problem yet -- of the top of my head, I feel like you might need to modify both the testing framework and nyc to get this functionality.

What I'd be tempted to do would be to set a global variable in the test suite (assuming it's not spawning any subprocesses) which would indicate the current name of the test that's running; I would then perhaps grab coverage before and after the test execution and diff the two objects -- I think this would be significantly easier than modifying the instrumenter to record this information ... it might however mean you end up writing a new module that mocha requires (or ava, tap, etc) rather than modifying nyc itself.

@stale
Copy link

stale bot commented Mar 7, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 7, 2019
@JaKXz
Copy link
Member

JaKXz commented Mar 7, 2019

I'm going to close this issue for now since it's been a while - while this sounds like a cool idea it might be outside the maintainable scope of nyc for now because of the inherent complexity. I would love to be proven wrong in a PR, or even draft PR :)

@JaKXz JaKXz closed this as completed Mar 7, 2019
@istanbuljs istanbuljs deleted a comment from stale bot Mar 7, 2019
@JaKXz JaKXz removed the wontfix label Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants