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

can no longer stub/spy (via sinon) on _super call in Ember 2.1, 2.2-beta.1 #12457

Closed
david-casagrande opened this issue Oct 7, 2015 · 5 comments

Comments

@david-casagrande
Copy link

this is obviously only relevant in testing and i haven't tested another stubbing library other than sinon.

example:

//class
export default ApplicationSerializer.extend({
  modelNameFromPayloadKey: function(key) {
    //manipulate key code
    return this._super(key);
  }
});

//test
test('#modelNameFromPayloadKey', function(assert) {
  var serializer = this.subject();
  var _superSpy = sinon.spy(serializer, '_super');

  serializer.modelNameFromPayloadKey('user');
  assert.equal(_superSpy.calledWith('user'), true, 'given user it passes user to `_super`);
});

i understand this example is a little awkward because you can run the assertion on what is returned from modelNameFromPayload, but I have a couple other instances where you Il manipulate an argument and confirm that the manipulation is correct and not what is actually happening after you call _super

as an additional bonus i have a mixin that has a method that call _super, that does works as expected if it's used in an Ember.Object that does not have the super method

@stefanpenner
Copy link
Member

Spying on _super isn't something we support, and as such I suspect this will be a "wont fix".

@mixonic
Copy link
Member

mixonic commented Oct 7, 2015

Agreed on wontfix. @david-casagrande I suggest simply using a different parent class, and asserting that the same-named method on the parent class is called.

@mixonic mixonic closed this as completed Oct 7, 2015
@rwjblue
Copy link
Member

rwjblue commented Oct 7, 2015

The _super behavior change was introduced in #12062.

The change that is causing this issue with spying is that in prior versions the value of this._super was always the same function for that object and it called this.__nextSuper which is the "upstream" function. After the PR above, we set this._super to the upstream function directly. This means that the spy that sinon is adding to this._super is replaced as soon as you call a function that is super wrapped.

That PR fixed a number of issues (and is much easier to debug) so it seems unlikely that we would revert this behavior change...

@david-casagrande
Copy link
Author

@rwjblue that's pretty much what i assumed was going on after looking through the CHANGELOG, but thx for confirming.

@mixonic that solution makes sense and will def work for my use cases

my only lingering concern is the behavior happening when spying/stubbing _super on class that doesn't have the method defined, but I guess that is more of a user error haha.

would there be any value added to throwing a warning to the user if they are making an unnecessary _super call?

@stefanpenner
Copy link
Member

It is also worth noting, es6 super is also unobservable in this way. Its basically an accident of having to implement this in user-space that it was even observable.

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

4 participants