Now spies preserve original function arity

[kapke]

Resolves #991
Sadly I needed to use eval for make spies working as expected.

[glebec]

I haven't tested it carefully yet but it looks good to me.

Sadly I needed to use eval for make spies working as expected.

Yes, but as you point out, there isn't much choice for dynamically making spies of a given arity. It would appear for example that Sinon uses hard-coded proxy functions limited to 12 arguments at most and they warn against using eval due to products that use it blocking that keyword. Do we have similar concerns? Which is the lesser of the two evils — limited arity or inclusion of eval? That is perhaps up to the project maintainers to choose.

EDIT: I am going to sleep now but I wonder if something along these lines might help.

[glebec]

What if we did:

var spy = /* full spy def., which can close over wrapper */,
    wrapper = (function(spy){ return Function(args, 'return spy.apply(this, arguments);') }(spy))

Then you get a "closure" (really just formal param from the IIFE) of the spy, and you also get a working this value.

[kapke]

I was trying with something like that before I used an eval. It looks that when function is created with new Function constructor, then it losts all of context in which it was created. So then I tried passing a spy as first argument, but then I went into troubles with preserving this which resulted the line above.

[glebec]

Dang. After playing around with it for a while, I now better understand exactly what you mean — Function does not create closures (different from what I call context, which I usually treat as a synonym to the this value). You can sort of force the spy inside the function body if you bind(spy) and use this.apply but then you can't change the binding of the returned spy. Basically it's just not possible to do without eval, as you already figured out. Or you can hard-code in the proxies like Sinon does but then you should add documentation to the effect of "Jasmine spies preserve arity up to length XX but not longer" or similar.

How frustrating.

[glebec]

Following our discussion, and doing a quick check I can see that eval is already in the Jasmine source, I would say this PR is ready IMHO.