benbria / coffee-coverage

Istanbul and JSCoverage-style instrumentation for CoffeeScript files.
MIT License
145 stars 31 forks source link

[Bug] The injected codes make the Promise(bluebird) can not work. #56

Open snowyu opened 9 years ago

snowyu commented 9 years ago

reproduce:

Q   = require 'bluebird'

readFile = Q.promisify (aPath, done)->
  if aPath == 'c'
    data = 'ok'
  else
    err = new Error('no such file')
  done(err, data)

Q.reduce ['a', 'b', 'c', 'd'], (content, file)->
  console.log file, content
  if !content
    content = readFile(file).caught(->)
  return content
, null
.then (content)->
  console.log 'content=', content

the result should be:

a null
b undefined
c undefined
d ok
content= ok

But after injected:

a null
b 0
c 1
d 1
content= 1
jwalton commented 9 years ago

I think I know what's going on here. As a workaround, if you change:

content = readFile(file).caught(->)

to

content = readFile(file).caught(-> undefined)

I bet it will work. I suspect what's going on here is we're instrumenting the empty function, adding a lineCount++ at the start, which means instead of returning undefined, it ends up returning the count.

snowyu commented 9 years ago

why should inject the code to empty function and return a value?

jwalton commented 9 years ago

Yeah, it's a bug for sure. The way coffee-coverage works is to instrument the original coffee-script before compilation, so we change:

->

to something like:

__coverageVar['filename.coffee].s[0]++
->
    __coverageVar['filename.coffee].f[0]++
    __coverageVar['filename.coffee].s[1]++

At the end of the test, we know how many times the 0th function and 1st statement were run. But, of course, this is wrong; it should be generating:

__coverageVar['filename.coffee].s[0]++
->
    __coverageVar['filename.coffee].f[0]++
    __coverageVar['filename.coffee].s[1]++
    return undefined
snowyu commented 9 years ago

It will solve the first problem in above list if so. But It still make the checker(isEmptyFunction) error.

The test whether empty function(second problem in the list) is important for my case too, My createObject function in the inherits-ex depends on it. this make it easier to write a class.

inherits = require 'inherits-ex'
createObject = require 'inherits-ex/lib/createObject'

class A
  constructor: ->console.log 'A'

class B
  inherits B, A

b = new B # the original way
b = createObject B # the constructor of A will be executed because the B's constructor is empty.

I have to work around the coverage's bug in my isEmptyFunction function temporarily.

jwalton commented 9 years ago

Hmm, interesting corner case. You can make a truly empty function with something like:

### !pragma no-coverage-next ###
x = ->

(We could add a ### !pragma no-coverage-block ### too, which would be a bit finer-grained.) But I'm not sure this is really going to help your use case. We could not instrument empty functions, but then you wouldn't know if your empty function was being called or not.

snowyu commented 9 years ago
class B
  inherits B, A

The empty constructor is generated by coffee-script compiler. So unless modify the compiler...

snowyu commented 9 years ago

maybe add a switcher to enable the measuring the calls of empty function.