Spying on instance methods

[takempf]

Inside some of my tests I've found that I need to spy on instance methods, but it seems that it's impossible at present since these are read only properties. When I try to use sinon's spy method...

var next_spy = sinon.spy( player, 'next' );

this happens:

TypeError: Cannot assign to read only property 'next' of #<Object>
    at Object.wrapMethod (C:\Users\Timothy\Repos\sync4\node_modules\sinon\lib\sinon.js:84:30)
    at Object.spy (C:\Users\Timothy\Repos\sync4\node_modules\sinon\lib\sinon\spy.js:228:22)
    at C:\Users\Timothy\Repos\sync4\test\db_room_model.js:102:26
    at C:\Users\Timothy\Repos\sync4\node_modules\orm\lib\Model.js:297:12
    at C:\Users\Timothy\Repos\sync4\node_modules\orm\lib\Singleton.js:32:10
    at Object.<anonymous> (C:\Users\Timothy\Repos\sync4\node_modules\orm\lib\Model.js:112:12)
    at C:\Users\Timothy\Repos\sync4\node_modules\orm\lib\Instance.js:24:7
    at Array.map (native)
    at Instance.emitEvent (C:\Users\Timothy\Repos\sync4\node_modules\orm\lib\Instance.js:23:17)
    at C:\Users\Timothy\Repos\sync4\node_modules\orm\lib\Instance.js:608:4

Is there a recommended method for mocking/spying on instance methods?

[dxg]

What exactly is next?

[takempf]

For what it's worth...

Room.Player = Room.extendsTo( 'player', {
    elapsed: {
        type: 'number',
        defaultValue: 0
    },
    duration: Number,
    order: {
        type: 'enum',
        values: ['normal','shuffle'],
        defaultValue: 'normal'
    }
}, {
    // TODO use this instead -> autoFetch: true,
    hooks: {
        afterLoad: function(){
            // TODO see if this can be mixed in to the instance
            if( !this.events ) this.events = new EventEmitter;
            this.timer;
        }
    },
    methods: {
        // select and play the next item
        next: function( cb ){
            cb = cb || function(){};
            var player = this;
            player.getRoom( function( err, room ){
                if( err ) return cb( err );
                room.getItems( function( err, items ){
                    if( err ) return cb( err );
                    if( items.length === 0 ){
                        player.load( null, function( err ){
                            if( err ) return cb( err );
                            return cb();
                        });
                        return;
                    }
                    var item;
                    if( player.order === 'normal' ) item = items[0];
                    if( player.order === 'shuffle' ) item = items[ _.random( items.length ) ];
                    room.removePlaylistItem( item.id, function( err ){
                        if( err ) return cb( err );
                        player.load( item, function( err ){
                            if( err ) return cb( err );
                            return cb();
                        });
                    });
                });
            });
        },
        ...
    },
    reverse: 'room'
});

[dresende]

I'm not sure if the next word is not reserved for iterators. You should change it before it bites you when ES6 arrives (without having to pass options to node executable).

[dresende]

I think this is because of this line:

https://github.com/dresende/node-orm2/blob/master/lib/Instance.js#L457

It defines the methods not specifying configurable (so the default is false), meaning it cannot be changed later. This is what sinon is trying to do. Please change it in your orm copy and see if it works. Something like this:

    for (k in opts.methods) {
        Object.defineProperty(instance, k, {
            value: opts.methods[k].bind(instance),
            enumerable: false,
            configurable: true
        });
    } 

If it works I'll update it. I don't want to touch the repository now since I'm waiting for some changes from a fork and don't want to cause conflicts.

[dresende]

It might be the writable option and not the configurable one.

[dresende]

If you still want this and the above change works please reopen and I'll change it.

[takempf]

@dresende the writable change fixes the issue for me.