Spying on instance methods #296

Activity

1. dxg commented on Aug 11, 2013

   

   dxg

   on Aug 11, 2013

   Collaborator

   More actions

   What exactly is next?
2. takempf commented on Aug 11, 2013

   

   takempf

   on Aug 11, 2013

   Author

   More actions

   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'
   });
3. dresende commented on Aug 12, 2013

   

   dresende

   on Aug 12, 2013

   Owner

   More actions

   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).
4. dresende commented on Aug 12, 2013

   

   dresende

   on Aug 12, 2013

   Owner

   More actions

   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.
5. dresende commented on Aug 12, 2013

   

   dresende

   on Aug 12, 2013

   Owner

   More actions

   It might be the writable option and not the configurable one.
6. dresende commented on Sep 13, 2013

   

   dresende

   on Sep 13, 2013

   Owner

   More actions

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

   closed this as completedon Sep 13, 2013
8. takempf commented on Sep 21, 2013

   

   takempf

   on Sep 21, 2013

   Author

   More actions

   @dresende the writable change fixes the issue for me.
9. dresende

   added a commit that references this issue on Sep 21, 2013

   Changes user methods to be writeable property instances (fixes #296)

   This issue will close once commit 36dd859 is merged into the 'master' branch.

   36dd859