Skip to content
This repository has been archived by the owner. It is now read-only.

optionally use node-cover when running npm test for backend code coverage data #2272

Merged
merged 1 commit into from Nov 1, 2012
Merged

optionally use node-cover when running npm test for backend code coverage data #2272

merged 1 commit into from Nov 1, 2012

Conversation

@jrgm
Copy link
Contributor

@jrgm jrgm commented Aug 13, 2012

An experiment with running backend tests with a code coverage tool. I want to convince myself that the numbers make sense, although so far they (mostly) do. But I thought I'd put this up here for people to ponder and try out (and for me to refer to when I file some issues where we are missing tests).

  • No additional setup is required beyond npm install and setting PERSONA_WITH_COVER=1
  • this will only have effect if PERSONA_WITH_COVER is set in the environment. At the end of a run, a ./cover_html directory is generated with nice annotated html reports per top-level or ./lib file used. Example: PERSONA_WITH_COVER=1 WHAT_TESTS=back npm test
  • Caveat on the html reports: the %lines number overstates coverage since it includes comments and blank lines. The %blocks is more useful in figuring out where more tests would be useful
  • uses node-cover which bundles esprima for js munging.
  • this PR alters run_locally.js to start the daemons wrapped in the cover command
  • also adding rimraf to packages.json to remove the generated .code_coverage directory between runs
  • to get coverage data done correctly for bin/router I had to register with require('../lib/shutdown').handleTerminationSignals
  • however when I tried do the same thing for handleTerminationSignals for bin/static, the tests started failing. I should look more into shutdown to figure this out. For now, bin/static usage of libs will be underreported.
  • running with PERSONA_WITH_COVER=1 costs 20% on wall-clock times, which is relatively cheap
  • currently back,back_mysql,front all pass for me either PERSONA_WITH_COVER or not. (Although running with WHAT_TESTS=front isn't particularly interesting probably. Also, some wsapi that are not called at all by back are called for only for failure cases with back_mysql (./tests/stalled-mysql-tests.js))
  • I've only run this on osx and linux (and win32 where I have a limping install going now).
@travisbot
Copy link

@travisbot travisbot commented Aug 13, 2012

This pull request passes (merged 72ee1cb9 into f257c08).

@travisbot
Copy link

@travisbot travisbot commented Aug 14, 2012

This pull request passes (merged f243012b into f257c08).

@ghost ghost assigned shane-tomlinson Aug 16, 2012
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Aug 22, 2012

@jrgm - One useful addition would be a separate scripts/test_backend_coverage script so this can be run without specifying extra environment variables while calling ./scripts/test.

Is there further work you want to do on this before final review?

@jrgm
Copy link
Contributor Author

@jrgm jrgm commented Aug 22, 2012

@shane-tomlinson I can look into rearranging this a bit. It's already an interesting weave of stuff though ;-)

I don't have anything to do add to this patch. I will rebase it later today.

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Sep 19, 2012

@jrgm If there's nothing else needed, I can do the rebase myself, since I'm sure you're a little busy :)

@jrgm
Copy link
Contributor Author

@jrgm jrgm commented Oct 19, 2012

Hey @zaach, can you look at these changes and check I didn't biff anything for PERSONA_DEBUG_MODE. And also the change to bin/router?

…rage data

To run 'PERSONA_WITH_COVER=1 npm test'.

Changes to lockdown.json are:
  rimraf@2.0.2
    -> graceful-fs@1.1.14
  cover@0.2.8
    -> underscore@1.2.4
    -> underscore.string@2.0.0
    -> cli-table@0.0.2
      -> colors@0.3.0
@jrgm
Copy link
Contributor Author

@jrgm jrgm commented Oct 30, 2012

Okay, I rebased again. Can someone please review this and merge (before another change to package.json/lockdown.json makes me do it again)?

@zaach
Copy link
Contributor

@zaach zaach commented Nov 1, 2012

@jrgm lgtm; no issues with routher or debug mode.

r+

zaach added a commit that referenced this pull request Nov 1, 2012
optionally use node-cover when running npm test for backend code coverage data
@zaach zaach merged commit 48c3c7a into mozilla:dev Nov 1, 2012
1 check passed
1 check passed
@ozten
default The Travis build passed
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants