search

alex-seville / blanket

blanket.js is a simple code coverage library for javascript. Designed to be easy to install and use, for both browser and nodejs.
http://blanketjs.org
Other
1.4k stars 177 forks source link

Tests using Knockout.js does not work correctly when instrumented with Blanket.js #291

Open mmanela opened 11 years ago

mmanela commented 11 years ago

A couples users on Chutzpah reported issues when generating code coverage with Blanket.js (which Chutzpah uses under the covers) for his code which uses Knockout.js. When run without coverage it executes fine and all tests pass. But when run with coverage enabled the following errors happen.

  1. Tests fail with

    ReferenceError: ko is not defined at load (http://localhost/cover/scripts/src/userDetailInfo.js:69:21) at Object.<anonymous (http://localhost/cover/scripts/tests/testwebsite.userdetails.test.js:52:24

  2. A global error of:

    Uncaught Error: Mismatched anonymous define() module: function E(w){function B(b,c,d){....

    Source:
    http://localhost/cover/blanket_qunit.js:6389

Although Chutzpah currently using version 1.0.4, I tested this issue with the most recent version of Blanket.js and the the same issue occurs. I created/simplified the users sample and pushed it here: https://github.com/mmanela/blanket-knockout-repro

Thanks!

alex-seville commented 11 years ago

Thanks for providing a sample. I've added this bug to the v1.1.5 worklog: https://github.com/alex-seville/blanket/issues/285

alex-seville commented 11 years ago

One thing I notice right away, you're "covering" the library files and the test file. Usually this is not intended. Are you sure you don't just want the source file(s) covered?

<script data-cover type="text/javascript" src="scripts/common/jquery-1.7.1.js"></script>
<script data-cover type="text/javascript" src="scripts/common/knockout-2.1.0.js"></script>
<script data-cover type="text/javascript" src="scripts/common/jquery.mockjax.js"></script>
<script data-cover type="text/javascript" src="scripts/src/userDetailInfo.js"></script>

<script data-cover type="text/javascript" src="scripts/tests/testwebsite.userdetails.test.js"></script>

should probably be:

<script type="text/javascript" src="scripts/common/jquery-1.7.1.js"></script>
<script type="text/javascript" src="scripts/common/knockout-2.1.0.js"></script>
<script type="text/javascript" src="scripts/common/jquery.mockjax.js"></script>
<script data-cover type="text/javascript" src="scripts/src/userDetailInfo.js"></script>

<script type="text/javascript" src="scripts/tests/testwebsite.userdetails.test.js"></script>
alex-seville commented 11 years ago

If this is automatically generated by chutzpah, it means you need to add a filter. I'm not sure how they've built that into Chutzpah, but basically something like:

<script data-cover type="text/javascript" src="blanket_qunit.js" data-cover-only="scripts/src/*" ></script>

should work.

mmanela commented 11 years ago

I had thought so too originally but I had tested that and it didn't matter. I updated the sample repo with a file named withCoverage2.html which shows this also. The same error occurs with that file

satyarapelly commented 11 years ago

Thanks Matthew and Alex for taking care of the issue. Thanks for your time. really appreciated.

alex-seville commented 11 years ago

Moving the <script type="text/javascript" src="blanket_qunit.js"></script> code after the jQuery and Knockout references fixes the issue.

As to why that's happening, I can't give a definitive answer right now. I think it's partially related to the syntax of the source file, and also to the fact that blanket, via requirejs, define or redefines define (which I suspect is used by JQuery or Knockout).

I'm not sure if you're able to make progress with this answer or not. I'll try to find out the exact reason.

mmanela commented 11 years ago

Good to know but I don't think I can use it since I autogenerate the html test harness I don't know which files should/should not go above the reference to blanket.

alex-seville commented 11 years ago

I figured. I'm going to work on it this week and try to find a proper fix.

Sent from my iPhone

On May 20, 2013, at 7:59 AM, Matthew Manela notifications@github.com wrote:

Good to know but I don't think I can use it since I autogenerate the html test harness I don't know which files should/should not go above the reference to blanket.

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

alex-seville commented 11 years ago

I've made some progress on this issue. The define function in our embedded Requirejs was conflicting with a define function in either jQuery or Knockout. I've actually replaced Requirejs in the Blanket code with custom code and it resolves that issue. The coverage numbers are still incorrect though, so I'll fix that as well.

mmanela commented 11 years ago

Ok that sounds good.

Also, How does Blanket.js interact pages that are using Require.js for loading?

alex-seville commented 11 years ago

Blanket overrides the requirejs loading function in that case. Once requirejs loads a file, blanket determines if it needs to be instrumented or not and if so returns the instrumented copy of the file, rather than the original source.

On Mon, May 27, 2013 at 9:46 AM, Matthew Manela notifications@github.comwrote:

Ok that sounds good.

Also, How does Blanket.js interact pages that are using Require.js for loading?

— Reply to this email directly or view it on GitHubhttps://github.com/alex-seville/blanket/issues/291#issuecomment-18506766 .

alex-seville commented 11 years ago

I've released v1.1.5, but I'm not sure it will resolve your issue entirely. Let me know.

alex-seville commented 11 years ago

Did the changes resolve your issue?

mmanela commented 11 years ago

Overall, it seems to have solved it but I do have a regression. Files that are loaded by a tests using require.js are no longer getting code coverage. I suspect this is do to your change of pulling require.js out of blanket. Is there something special I must do to make those still work?

alex-seville commented 11 years ago

It should still work because I left the requirejs loader code intact. The blanket script reference will need to be included after the requirejs reference but before the requirejs initialization to be hooked in properly.

oiva commented 11 years ago

Maybe that could be better explained in the documentation? It took some hours of work trying to get requirejs and blanket working together before noticing this comment.

alex-seville commented 11 years ago

Agreed. I think a note somewhere explaining the ordering would be useful.

On Fri, Aug 9, 2013 at 3:23 AM, Oiva Eskola notifications@github.comwrote:

Maybe that could be better explained in the documentation? It took some hours of work trying to get requirejs and blanket working together before noticing this comment.

— Reply to this email directly or view it on GitHubhttps://github.com/alex-seville/blanket/issues/291#issuecomment-22386504 .

nicoleta-scrimint commented 10 years ago

If the issue was solved for BlanketJs, what should a user of Chutzpah do to make Knockout work when running code coverage? Should there be done a specific configuration in the file chutzpah.json?

Thank you for your support.

satyarapelly commented 10 years ago

Hi,

From Chutzpah side, there is no change. you need to get the latest Chutzpah version. It working good.

thanks

On Sat, Jun 28, 2014 at 3:45 AM, nicoleta-scrimint <notifications@github.com

wrote:

If the issue was solved for BlanketJs, what should a user of Chutzpah do to make Knockout work when running code coverage? Should there be done a specific configuration in the file chutzpah.json?

Thank you for your support.

— Reply to this email directly or view it on GitHub https://github.com/alex-seville/blanket/issues/291#issuecomment-47424238 .