wardbell / bardjs

Spec helpers for testing angular v.1.x apps with Mocha, Jasmine and QUnit
MIT License
178 stars 34 forks source link

bardJS can't work well with JasmineJS+RequireJS+AngularJS #5

Closed carson-song-lsq closed 9 years ago

carson-song-lsq commented 9 years ago

Hi Ward Bell,

I like your effort on this, i'm trying to use bardJS to optimize our unit test(Jasmine+RequireJS+Angular), but get stuck by some issues.

When i try to use bard.inject(this, '$q') in beforeEach-block, it throw errors.

  1. First error is that "sinon is not defined", so i have to download and refer SinonJS. Regarding to this, could you please update the documentation how to install and use bardJS? This is kind of hidden dependency....
  2. After i fixed the error above, i met another error: Error: [$injector:unpr] http://errors.angularjs.org/1.3.4/$injector/unpr?p0=%5Bobject%20Object%5DProvider%20%3C-%20%5Bobject%20Object%5D at Error (native) at Object.bardInject [as inject]

By debugging, i found the error happened in bard.js, is from here: angular.forEach(args, function(name, ix) { var value = $injector.get(name); //args=[this,'$q'] in my case. So, the args is supposed to remove the first argument this, right? Why it's not?

Do you have any insight about this?

Thanks, Carson Song

carson-song-lsq commented 9 years ago

To make it work, I did a quick fix as below. This might not be a good solution for the root cause, can you take a look at it?

function bardInject() {

//line 301 var names = []; angular.forEach(args, function (name, ix) {

        //myFix Start
        if (!angular.isString(name)) { return; }
        //End of Fix

        var value = $injector.get(name);
        if (value == null) { return; }
wardbell commented 9 years ago

Hello Carson Song.

Thanks for your report and your interest in bard.js.

You have identified two problems in the current version (v.0.1.2).

  1. the dependence on sinon.js which isn't documented
  2. the signature of bardInject when its first arg is this

sinon dependence

I should mention it in the README ... and I will. I'll also tell people to include it among the scripts loaded for their tests.

FWIW, the bower.json and package.json clearly announce the dependence on sinon. Did you use bower or npm to install bard. Those are the instructions. If not how did you get it?

I imagine you simply grabbed the library from the repo ... perhaps because you wanted to use Jasmine instead of Mocha.

this in bardInject

bardInject would have stripped the this from the front of your argument list ... if you were using mocha. That's what getCtxFromArgs does ... or tries to do. getCtxFromArgs doesn't really know what the first arg is so it looks for a clue ... and the clue it is looking for is a mocha method called test. If it doesn't see that member, it doesn't remove the first arg (the this in your case) from the args list.

But you're using jasmine not mocha. You really shouldn't be passing this to bardInject. Of course it is easy to see why you did ... because I told you to do so.

That's something to clarify too.

Resolution

I just made a bunch of documentation changes and some code changes so that you won't be hurt running Jasmine even if you DO put this in your arg list.

I still recommend that you remove it.

I've pushed these changes as v.0.1.3.

Please try them. I haven't run Jasmine with bard in a long time ... and I don't have an easy way to do so right now.

Appreciate the help!