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

Instrumentation error, you cannot redefine the 'window' variable #311

Closed
provegard opened this issue Jul 15, 2013 · 9 comments
Closed

Instrumentation error, you cannot redefine the 'window' variable #311

provegard opened this issue Jul 15, 2013 · 9 comments

Comments

@provegard
Copy link

This is not so much an error report, more of a question. I get the error message above when instrumenting Angular 1.0.5. Angular contains the following function:

function setupModuleLoader(window) {

What's the rationale for not accepting this?

@alex-seville
Copy link
Owner

There was a PR a few months ago submitted by another user. The intention was to ensure that the coverage variable (stored in window) wasn't overriden by the code being covered. Recently I was reviewing that code and decided that it's not really the responsibility of Blanket, so it's going to be removed in version 2.0 (whether or not it's handled by Istanbul is not clear, but I suspect it isn't).

@provegard
Copy link
Author

Ok, thanks for the answer! As I said, just a question so I'll close the issue.

@BeheadedKamikaze
Copy link

+1 if this can be removed as we use AngularJS also and this is blocking us from keeping our test tooling at the current version. Thanks for raising the issue though; I think it's important for others wondering why it doesn't work with AngularJS any more.

@alex-seville
Copy link
Owner

I'm interested to know why Angular is redefining window as well (if it actually is)...

@BeheadedKamikaze
Copy link

Well... it is and it isn't :) It's declaring a parameter to a function and naming it window (presumably so that it can be unit tested) but it's not actually overwriting window or explicitly declaring a new variable with that name in any global sense. It's possible they could have used a different name, but I don't think it's unreasonable to call it window as that's what it represents and also where its value comes from.

Likewise one is free to declare a parameter called 'undefined' (and not provide a value for it in the call to the function) to ensure that if somebody else has overwritten it, you still get the 'real' value of undefined.

But for what it's worth, I think you are right about it not being the responsibility of blanket.js to raise it as an error. It's more of a coding style issue where one's team could come to its own agreement about whether it is or isn't acceptable to name variables the same as browser objects, and be consistent with that choice.

Edit: minification might be another reason to bring window in as a parameter to a function, so that its name would be shortened throughout the file.

@feelingweird
Copy link

+1 would love to use this with Chutzpah and Angular,

@windump
Copy link

windump commented Aug 29, 2013

How to get Chutzpah Code Coverage in version 2.4.3 with AngularJS.

After installing 2.4.1, VS would not show the context menu for running tests, only 2.4.3 was "usable" but code coverage does not work with angularJS.

So I saved the blanket_qunit.js (173 KB‎); blanket_jasmine.js‎ (173 KB‎) files from 2.4.1, reinstalled 2.4.3, then copied them into this folder:

Application Data\Microsoft\VisualStudio\10.0\Extensions\Matthew Manela\Chutzpah - A JavaScript Test Runner\2.4.3\TestFiles\Coverage

Code coverage works perfectly now.

@BeheadedKamikaze
Copy link

Yes that should work. Also if you have Visual Studio 2012 and you're using the Chutzpah Test Adapter (as opposed to the Test Runner) the path will be:

%localappdata%\Microsoft\VisualStudio\11.0\Extensions\JSRunners

Visual Studio 2012 will generate a random each time you install it. Just look for the one with chutzpah.dll.

I haven't tried this, but it might also work with 2.4.3 if you reference the minified version of Angular, i.e. angular.min.js because the minified variable will be called something else instead of 'window'. I am not able to try it at the moment because our dev environment needs to be fairly stable before a release, but let me know if you get this to work.

@simoncalvin
Copy link

Referencing the minified version of angular works.

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

No branches or pull requests

6 participants