Last Comment Bug 789401 - cfx test should support a --coverage option
: cfx test should support a --coverage option
Status: RESOLVED WONTFIX
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks: cfx.py
  Show dependency treegraph
 
Reported: 2012-09-06 23:21 PDT by Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )
Modified: 2013-12-18 09:12 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-06 23:21:53 PDT
possible ways in:

http://esprima.org/doc/#unittests
https://github.com/itay/node-cover#readme
Comment 1 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-07 00:06:44 PDT
The meat of it seems to be:

* change `require`, such that if it's a file we care about, it gets instrumented.
  (https://github.com/itay/node-cover/blob/master/index.js#L343)
* read in the file, parse it using esprima, instrument it (https://github.com/itay/node-cover/blob/master/instrument.js#L111), and remake it using escodegen.


https://github.com/arian/CoverJS  takes a different approach:  here you copy in 'instrumented' versions of the code, then just run your tests on that.  Since we are copying around files anyway, perhaps this is an attractive option?

This is a lot of work to implement, but would help *a lot*.  Plus, coverage reports look cool.
Comment 2 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-07 09:39:40 PDT
Escodegen and esparsa don't know the full moz, dialect, notably things like 

   const {a,b,c} = {}  /*  "ObjectPattern" */

A fine solution would be to push those changes up into Escodegen and Esparsa.  Spidermonkey's "Reflect.parse()" gets these right, but it would be nicer to have a pure js solution.
Comment 3 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-10 10:58:21 PDT
So far this has been quite tedious to work on :)  

Neither Esprima nor escodegen have fully finished ports.  I have a very sad face.

CoverJS is very promising, as a runner.
Comment 4 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-13 13:38:36 PDT
I am pushing all the code up into esprima#harmony.  I have been running my changes over the addon-sdk code to find warts as well, and would love to share results.  In the meanwhile, I am happy to own this.
Comment 5 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-19 07:30:37 PDT
Most of the missing features have gone upstream into esprima, and for the others, I am maintaining.  I am working on getting escodegen up to snuff, but I am still not sure of the best way to get the actual instrumentation to happen.

GL
Comment 6 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-09-19 15:03:50 PDT
Now I am deep in the bowels of cfx, harness-options.js, packages/api-utils/lib/addon/runner.js and the like.... where can one actually hook in here, to either change Require, or keep track of some global object to keep the counts?
Comment 7 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-10-16 09:25:58 PDT
Progress report:  https://gist.github.com/3900355
Comment 8 [github robot] 2013-01-07 14:04:19 PST
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fa2458dfa1821ab75f58519088a73cd055a30b17
CoverObject, work in progress (bug 789401)

Discussion of method and approaches:

    https://bugzilla.mozilla.org/show_bug.cgi?id=789401

Recipe for usage at:

    https://gist.github.com/3900355

https://github.com/mozilla/addon-sdk/commit/2b94404eec571cafcc3c856caffaa4b29c5b52eb
Merge pull request #624 from gregglind/coverage

Bug 789401 - CoverObject, work in progress r=@gozala
Comment 9 Wes Kocher (:KWierso) 2013-08-27 01:01:29 PDT
I'm going through the list of open bugs that github robot has commented in. Is this bug fixed, Erik?
Comment 10 Erik Vold [:erikvold] (please needinfo? me) 2013-08-27 01:23:40 PDT
(In reply to Wes Kocher (:KWierso) from comment #9)
> I'm going through the list of open bugs that github robot has commented in.
> Is this bug fixed, Erik?

Doesn't look like it.
Comment 11 Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2013-08-27 10:43:44 PDT
My last attempt on this failed to make much progress.  Before, it relied on global 'coverage' object that all tests could see.  Alternative paths on this are welcome!
Comment 12 Dave Townsend [:mossop] 2013-11-19 10:54:27 PST
I want to chat about this at triage this week, we have an number of pull requests open for this and we need to figure out what to do with them.

Dave
Comment 13 Jeff Griffiths (:canuckistani) (:⚡︎) 2013-12-05 11:03:34 PST
as requested
Comment 14 Dave Townsend [:mossop] 2013-12-18 09:12:12 PST
At this point we don't have a decision on how to proceed and this isn't important enough for the SDK team to spend time on compared to other work. I'm going to close this. If the way forward becomes clearer in the future then we can reopen and talk about what taking a patch would look like.

Note You need to log in before you can comment on or make changes to this bug.