tj / should.js

BDD style assertions for node.js -- test framework agnostic
MIT License
2.75k stars 194 forks source link

Issue with eql properties order #129

Closed plus- closed 11 years ago

plus- commented 11 years ago

This is what I get when running should.eql() on an object before and after pushing it into mongo:

image

It looks like the properties are inverted, but I actually expected the eql() to be true here. Is this normal?

btd commented 11 years ago

Could you post your full test case? In normal case object keys unordered. Also which version of should.js do you used?

plus- commented 11 years ago

I'm using 1.2.2

The test case is not easy to post but basically does something like:

var user = new User({ username: 'Luna' });
var post = new Post({ _user: user._id, date: new Date() });

describe('#read()', function() {
  before(function(done) {
    post.save(done);
  });

  it('should load object properties', function(done) {
    Post.findById(post._id, function(err, obj) {
      obj.should.eql(post);
      done();
    });
  });

});

where User and Post are mongoose models.

btd commented 11 years ago

I have reproduced this case:

var mongoose = require('mongoose');
mongoose.connect('mongodb://localhost/test');

var Cat = mongoose.model('Cat', { name: String });

var kitty = new Cat({ name: 'Zildjian' });

kitty.save(function (err) {
  Cat.findById(kitty.id, function(err, obj) {
    obj.should.be.eql(kitty);
  });
});

And it of course throw exception:

AssertionError: expected { name: 'Zildjian', _id: 52381a5c757c4d26b1000001, __v: 0 } to equal { __v: 0, name: 'Zildjian', _id: 52381a5c757c4d26b1000001 }

But actually it is bug of mongoose how it works with properties. How we can compare object - by keys:

var kitty = new Cat({ name: 'Zildjian' });

var test = { _id: new ObjectID(), name: 'Zildjian' };

console.log(Object.keys(kitty)); // [ '$__', 'isNew', 'errors',  '_maxListeners',  '_doc',  '_pres', '_posts', 'save' ]
console.log(Object.keys(test)); // [ '_id', 'name' ]

As you see mongoose make fields not enumerable. So if we compare two save from different objects we use ==:

// 7.4. Other pairs that do not both pass typeof value == 'object',
  // equivalence is determined by ==.
  } else if (!isObject(actual) && !isObject(expected)) {
    return actual == expected;
plus- commented 11 years ago

Yes the save key is irrelevant because it's a function.

Can't we filter out the function properties in the _deepEqual() ? I know it's not strict equality but we are talking about eql() so I suppose people just want to test non function properties.

btd commented 11 years ago

I think mongoose uses Object.defineProperty to define getters and setters. If they make it enumerable we cannot make common case. I suggest you use model.toObject() for this case:

console.log(kitty.toObject()) // { name: 'Zildjian', _id: 523871f79e3777ddc2000001 }

obj.toObject().should.be.eql(kitty.toObject());
plus- commented 11 years ago

I suppose that will do. Thanks btd