Function Coverage

[abramsh]

I saw on the to-do list a mention of function coverage.

This was something I was interested in, so I created a rough implementation; are you interested in a donation so that you can take it the rest of the way? What I have works, but I'm sure I missed some corner cases, etc.

BTW, I started on this because what I'm really interested in is per-function path coverage; any chance that can be added to the to-do list?

[tntim96]

Sure. Why not fork JSCover on GitHub and add your changes. I'm happy to take look.

Once you've got something workable, we can look at merging it back in.

[ghost]

Sorry for the delay, and for replying from a new GitHub account.

I've fork'ed and added my changes. Please take a look and let me know if you think it's merge-worthy: https://github.com/howard-abrams/JSCover

[tntim96]

Sure

[tntim96]

The code changes that are there look OK, but I just ran it on YUI3, converted to LCOV and can't see any function data.

From the code and running the tests I can see the following still needs implementing:

• Function data in in-built 'jscoverage.html' report is missing (this is a bit tricky)
• No acceptance tests (due to point above)
• A few unit tests missing here and there (I need to review a bit more)
• Switch to turn function coverage on/off (easy)
• Update the manual which can be done once the above is complete (easy)

The 'jscoverage.html' UI is a bit crowded for space, so it would be good to make function data inclusion conditional. There's currently no JS code to do this - the branch data is always displayed, so that could be even trickier.

I'll try to fork you fork - and see if I can send pull requests across.

[ghost]

Let me know how it goes on the fork'd fork.

I intentionally left off jscoverage.html and the config switch until you saw what I had done. While not "complete" perhaps the function coverage can be added back into the trunk with the caveat that it's only visible via LCOV for now.

LCOV should be working. My test case was to collect coverage from multiple files/runs, merge them, run the LCOV report, and then generate an html report from that using jgenhtml - seems to work. Also, the unit tests do some (minimal) coverage of that functionality.

[tntim96]

Let me know how it goes on the fork'd fork.

Having looked at this, I think it'd be easier to add me as a committer to your fork.

LCOV should be working

I can't see any function data in the JSON from my test either. jscoverage.js isn't modified. Are you sure you've committed everything?

[tntim96]

BTW - I've made a minor change you should fetch. I noticed the instrumented code appearing in the branch source alert, which should now be fixed.

[ghost]

I've made you a collaborator.

I didn't modify jscoverage.js at this point; I assumed that was only used for the jscoverage.html report, no?

As far as I can tell, everything is checked in, are you sure you built it? :)

I don't doubt that I've missed something here, and sorry for all the questions, but...

do you see the changes in NodeProcessor and SourceProcessor being called?
Do you see references to "functionData" at all in the jscoverage.json or instrumented source from your test run?
Does the "InstrumentAndHighlightRegressionTest" pass for you? (and if so, do you see the function instrumentation in the expected javascript-function js?)

[tntim96]

I've made you a collaborator.

Thanks.

I didn't modify jscoverage.js at this point; I assumed that was only used for the jscoverage.html report, no?

jscoverage creates the JSON that is sent to the server, so I'm not sure how you could get the functionData in the JSON without it. It's easy to add though.

Do you see references to "functionData" at all in the jscoverage.json

Yes, but only incorrect references from an existing bug that I fixed in the last check-in.

[tntim96]

I rebased your repository to pick up the latest changes and added a test.

Also, before we get too far, we need to sort out the source credits. I'm happy to leave them in (I may alter the format a bit - possibly to JavaDoc), but "Function Coverage added by Howard Abrams..." will need to include me on files I touch or write tests for (I consider untested code incomplete). I'll certainly credit you on the web-site and the Git history should reflect your contributions after the merge. Let me know if you're OK with this.

[tntim96]

I've pretty much made all the changes I think need to be added except for updating the mannual. Please review and let me know if you're happy for me to merge back to the main repository.

[ghost]

Thanks - and sorry, I thought I had replied to your previous comment.

I took a look through your changes and didn't see anything concerning, and I rebuilt and everything in-my use-case seems to still be working for me. If you're happy with things, then yes, please merge it back.

[tntim96]

Done. Your commits are in the history. Will do some final tidying and release in a day or two.