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

Full coverage suite with istanbul #65

Merged
merged 19 commits into from
May 18, 2015
Merged

Conversation

blinkdog
Copy link
Contributor

The source of all the micro-issues in #61 ...

A full (98+%) coverage suite for the statements, branches, functions, and lines of rot.js.

Instructions for generating the coverage report in node/README.md; requires Node.js.

Patrick Meade added 19 commits May 9, 2015 05:29
Added some additional ignore paths to .gitignore and .hgignore
Created Cakefile to provide test coverage infrastructure
Added devDependencies to package.node, to facilitate test coverage
Created node/test directory to hold test source files (CoffeeScript)
Created coverage tests for node-shim.js, rot.js, and text.js
Created some Node-specific documentation in node/README.md
Created coverage tests for array.js, number.js, string.js, object.js, function.js, and raf.js
Created coverage test for display.js
Created coverage test for backend.js, rect.js
Created coverage test for hex.js, rect.js
Created coverage test for tile.js
Modified node-shim.js to include stub for clearRect()
Created coverage test for rng.js
Created coverage test for stringgenerator.js
Created coverage test for eventqueue.js
Re-organized test source files into directory structure that matches src/
Updated paths to support new folder structure
Fixed RAF-loop bug in display/displayTest.coffee
Annotated display/rectTest.coffee with no-coverage method placeholder
Created coverage test for scheduler/scheduler.js
Annotated node/nodeShimTest.coffee with no-coverage method placeholder
Created coverage test for engine.js
Created coverage test for scheduler/scheduler-action.js
Created coverage test for scheduler/scheduler-simple.js
Created coverage test for scheduler/scheduler-speed.js
Annotated display/displayTest.coffee with no-coverage method placeholders
Fixed errant console.log statements left in engineTest.coffee
Added coverage for clearRect() in node/nodeShimTest.coffee
Added missing coverage for _tick and _draw methods of Display
Added missing coverage for _drawNoCache method of Rect
Added coverage tests for src/map directory
Updated istanbul and mocha versions in package.node
Added coverage tests for src/noise
Modified rngTest.coffee to be a little less picky
Added coverage tests for src/fov
Added coverage tests for color.js
Added coverage tests for lighting.js
Modified rngTest.coffee to not be so picky about RNG seeding
Modified colorTest.coffee to not fail due to unclamped random colors
Added coverage tests for src/path
Added coverage tests for Node.js's "term" terminal layout
Final cleanup of coverage suite after merge from ondras/master
@ondras
Copy link
Owner

ondras commented May 18, 2015

Wow, impressive!

I have no idea how exactly this works, but it looks like you need to provide (duplicate) server-side unit tests to have almost all lines of the code execute?

Those coffeescript unit tests look suspiciously similar to some of those existing jasmine-based browser tests. Is there a way to reduce this duplicity?

I will merge this anyway, of course.

ondras added a commit that referenced this pull request May 18, 2015
Full coverage suite with istanbul
@ondras ondras merged commit eefd078 into ondras:master May 18, 2015
@ondras
Copy link
Owner

ondras commented May 18, 2015

Thanks for this tedious work!

@blinkdog
Copy link
Contributor Author

I have no idea how exactly this works, but it looks like you need to provide (duplicate) server-side unit tests to have almost all lines of the code execute?

CoffeeScript has a Cakefile, which is similar to a Makefile, except it's just a CoffeeScript source file with a few helper functions.

cake coverage tells the cake executable to kick off the coverage task, which in our Cakefile is defined as:

task 'coverage', 'Perform test coverage analysis', ->
  clean -> compile -> test -> coverage()

Which is really just cute syntatic sugar for this (JavaScript):

task('coverage', 'Perform test coverage analysis', function() {
  return clean(function() {
    return compile(function() {
      return test(function() {
        return coverage();
      });
    });
  });
});

So first we clean: exec 'rm -fR test'

Then we compile: exec "node_modules/.bin/coffee -o test/ -c node/test"

  • This turns all of our CoffeeScript tests in node/test into JavaScript tests in test

Then we test: exec 'node_modules/.bin/mocha --colors --recursive'

  • This tells mocha to go run the test suite (by default it looks in the test directory)
  • Mostly this is for development purposes; it's not strictly necessary for a coverage report.

Finally we coverage: exec 'node_modules/.bin/istanbul cover node_modules/.bin/_mocha -- --recursive'

  • This tells istanbul to instrument the code, so we can figure out which lines, branches, functions, etc. get taken/called or not.
  • istanbul then uses mocha to run the test suite.
  • After running the tests, it collects the data generated from the instrumented code, and generates a report at coverage/lcov-report/index.html.
  • The coverage task also opens the report with firefox: firefox --new-tab coverage/lcov-report/index.html

In the report, you'll see every line, every branch, every function that did/did not get executed.

I organized the test suite according to the source tree. That is, node/test/display/displayTest.coffee corresponds to src/display/display.js. This was to make it easier for developers to know where the tests for a specific piece of code live.

The tests are also organized as follows:

  • describe "file"
    • describe "Class"
      • describe "Constructor"
      • describe "Method_1"
      • describe "Method_2"

This was so that finding the specific tests for a very specific part of the source would be easy as well.

Between the report itself and studying the code to figure out how to write a test that would cover each line, each branch, each function, etc. that's why I was able to find so many micro-issues.
Unreachable lines, if statements where it's impossible to reach the else case, etc. stick out like sore thumbs as one writes the necessary tests! :-)

Those coffeescript unit tests look suspiciously similar to some of those existing jasmine-based browser tests. Is there a way to reduce this duplicity?

Yes, mocha suites are very similar to jasmine suites:

The biggest differences (that I've noticed), are that mocha doesn't give you expect, but rather makes you choose your own assertion library, and it doesn't have the nice spy/mocking that jasmine does.

I'd have to study it a little more, but there's probably a way to get the mocha tests to run under the jasmine spec runner. We'd have to figure out where everything should live (and make some nice targets for the Makefile), but we could reduce some of the duplicate tests that way.

I need a short break from writing tests for awhile, but I'll start looking at ways to combine/improve the testing, maybe around the start of June.

All in all, I think it was worth it! :-)

@blinkdog blinkdog deleted the node-coverage branch May 19, 2015 14:56
@ondras
Copy link
Owner

ondras commented May 19, 2015

Thanks for this complex explanation. One really learns new stuff every day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants