Coverage in addon files not working in ember-cli 1.13.13

[offirgolan]

In ember-cli 1.13.12, I had ~95% coverage but once I upgrade to ember-cli 1.13.13, it dropped to ~50% with no other changes. What I noticed, is that all lines in my addon tests were red, but tests for files in my dummy app were working fine.

Coverage: https://codeclimate.com/github/offirgolan/ember-cp-validations/coverage
Code Climate Feed: https://codeclimate.com/github/offirgolan/ember-cp-validations
Repo: https://github.com/offirgolan/ember-cp-validations

You can check out the blanket-options.js file there as well as the npm and bower dependencies.

[sglanzer-deprecated]

I noticed this yesterday as well after upgrading a number of addon repositories to 1.13.13 - I originally thought my code was at fault, but that wasn't the case (I stepped through line by line and everything was being executed).

My current theory is that it's somehow related to this change https://github.com/twokul/ember-cli-release-notes/commit/bd5ac542c0d6dd8e095553d6528ec40ae4be6b4e, but it's a very loose theory.

[sglanzer-deprecated]

Doesn't appear to be that theory - looking into ember-cli-test-loader now

[offirgolan]

👍 thanks for looking into this!

[sglanzer-deprecated]

No problem, was causing me headaches as well

[sglanzer-deprecated]

I built a simple addon project and debugged the coverage - during the debugging it appears that the tests have already executed before the instrumentation is done.

So it looks like the test runner isn't being delayed. Gotta dredge up some old junk from my brain to remember what to do about that...

[offirgolan]

Interesting... how come it worked for tests in dummy/* but not for tests on file in addon/*?

[sglanzer-deprecated]

Just a theory again, but there is the split between loading the app and the tests - probably plays a role in this. From the tests/index.html:

<script src="assets/vendor.js"></script>
    <script src="assets/test-support.js"></script>
    <script src="assets/dummy.js"></script>
    <script src="assets/blanket-options.js"></script>
    <script src="assets/blanket-loader.js"></script>
    <script src="testem.js" integrity=""></script>
    <script src="assets/tests.js"></script>
    <script src="assets/test-loader.js"></script>

I've tried changing order, but no luck so far

[sglanzer-deprecated]

As far as I can tell the only parts of the addon files that are "covered" are the loading of the module itself - the rest of the code has already been executed by the time the coverage lines are put in place.

[jschilli]

I'll try this in a mocha project tomorrow. I have one or two addons that are 1.13.13 and don't recall seeing this

-jeff

On Dec 10, 2015, at 8:37 PM, Steven Glanzer notifications@github.com wrote:

As far as I can tell the only parts of the addon files that are "covered" are the loading of the module itself - the rest of the code has already been executed by the time the coverage lines are put in place.

—
Reply to this email directly or view it on GitHub.

[sglanzer-deprecated]

I'm running all of my addons with mocha currently and the test project I just put together was qunit - it's happening in both unfortunately.

[sglanzer-deprecated]

I turned autostart off to observe require order - all tests and jshints are required and instrumented prior to the start being called, then when the tests are running the dependencies are required and instrumented. It appears at this point the instrumentation happens after the test is run. I can't recall how this worked before so I'm going to set up a 1.13.8 addon and run the same sequence.

[sglanzer-deprecated]

1.13.8 results:

1. Same behavior for loading tests and jshints
2. Tests load dependencies during execution and wait for instrumentation before proceeding
3. Coverage works as expected

So at this point we have a handle on what the issue is at least.

I have to take off for tonight, I'll try to dig at this more tomorrow - @rwjblue and @stefanpenner any insights on the change in behavior?

[sglanzer-deprecated]

The difference to note would be between the 1.13.13 screenshot and 1.13.8 screenshot in terms of the test execution (done vs. paused at the component test)

[jschilli]

I got some debugging in on @offirgolan's project. The regression was introduced in loader.js (https://github.com/ember-cli/loader.js/commits/master) between 3.3.0 and 3.4.0

There was a fairly substantial refactoring internally - my suspicion is that the 'global' require that we're patching is now shrouded for certain code paths.

There was an error message reported in #85 that I cleaned up when we monkey patch require. (to no avail and expecting none)

In short, the dependencies of the tests (at least those in the addon) are going down a different code path and never being instrumented

Will try to carve out some time for this tomorrow, but up against some hard deadlines - so other thoughts welcome

[jschilli]

I think it's this line: https://github.com/ember-cli/loader.js/pull/54/files#diff-73f82623658278cf03c2acf12426f916R86

loader.js is using findModule internally to resolve 'defined' modules - it used to read

reified[i] = requireFrom(resolve(dep, name), name);

and now does:

reified[i] = findModule(resolve(dep, this.name), this.name).module.exports;

still not sure what to do about it but naively replacing that line with

          reified[i] = require(dep);

puts the coverage back to > 94%

cc: @stefanpenner - any thoughts on making our require hack safer? supported?

[stefanpenner]

Do you guys just need to intercept all requires? I can likely make a hook or something, if someone gives me a TL;DR

[jschilli]

Yes and the import equivalents. At least on the first time seen. We annotate the imported code with coverage probes before returning the resolved/required code

On Dec 11, 2015, at 5:27 AM, Stefan Penner notifications@github.com wrote:

do you guys just need to intercept all requires??

—
Reply to this email directly or view it on GitHub.

[stefanpenner]

@jschilli can you link me to where that is done currently?

[jschilli]

https://github.com/sglanzer/ember-cli-blanket/blob/master/vendor/blanket-require.js#L74 @stefanpenner

[sglanzer-deprecated]

Try not to freak out at the ugly dependency code =)

[raido]

I upgraded from Ember CLI 1.13.6 to 2.2.0-beta.6 and coverage dropped from 100% to 22%.

[jschilli]

Can you retest this with version 0.9.1 (and be sure to update blanket.js via ember g ember-cli-blanket) and reopen if it still persists

[crudo1f]

nice. that fixed the issue for me. thx.