test/local/log_tests.js depends on code coverage in order to succeed (which is a fail ;-) )

[jrgm]

STR:

1. checkout fxa-auth-server and npm install

2. Try any of:

   • npm test
   • NODE_ENV=dev CORS_ORIGIN=http://foo,http://bar ./scripts/tap-coverage.js test/local test/remote 2>/dev/null
   • NO_COVERAGE='' NODE_ENV=dev CORS_ORIGIN=http://foo,http://bar ./scripts/tap-coverage.js test/local test/remote 2>/dev/null

   Those all pass (they're essentially the same command).

3. Now, without code coverage on:

   • NO_COVERAGE='1' NODE_ENV=dev CORS_ORIGIN=http://foo,http://bar ./scripts/tap-coverage.js test/local test/remote 2>/dev/null

   That fails for test/local/log_tests.js.

   Running that test outside of tap-coverage, also fails:

   • NODE_ENV=dev CORS_ORIGIN=http://foo,http://bar ./node_modules/.bin/tap test/local/log_tests.js 2>/dev/null
   • NODE_ENV=dev CORS_ORIGIN=http://foo,http://bar node test/local/log_tests.js 2>/dev/null

   Again, these all fail test/local/log_tests.js.

4. Huh?

/cc @vbudhram, @philbooth

p.s., go away for a few days, and github switches to an ugly font!

[jrgm]

node 0.10.46, by the way.

[jrgm]

Fails the same way with node 4.4.7

[philbooth]

This is my fault and because of a clash between how proxyquire works and how our instrumented coverage works.

proxyquire mandates that you give it the required paths relative to the module under test and normally that would be fine. However, when running against the instrumented code, that relative path ends up different to what it is in normal execution.

Concretely, for the tests to run correctly uninstrumented, line 33 of test/local/log_tests.js should look like this:

mocks['./metrics/statsd'] = function () {

But that fails for the instrumented code. For the tests to run correctly instrumented, line 33 instead looks like this:

mocks[path.resolve(__dirname, '../../lib') + '/./metrics/statsd'] = function () {

Both when I wrote these tests originally and also again just now, I spent a bit of time trying to come up with some kind of mock path that works for both cases. I failed, so I chose the option that worked when you run npm test.

Unless someone else does better at that than I did (🎉🍻 if you do), it may be worth investigating whether an alternative to proxyquire such as mockery can give us a quick win. Failing that, I guess we need to look at a different approach to coverage? I see that we use something called ass, but I don't know anything about how it works. I have successfully used istanbul with mocking libraries without any problems in the past though, if we're looking for alternatives.

[jrgm]

Ah, thanks.

So, we can ultimately change that to mocks['./metrics/statsd'] = function () {, and that will then pass, either when run with tap, or node, or with nyc. Done.