1069150 - [sms] unittest threads_test.js incorrect using assert.include for testing existing object property

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[sv99]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20140917114326

Steps to reproduce:

253: assert.include(Thread.prototype, 'drafts'); 

assert.include - test inclusion of an object in another. Works for strings and Arrays, for other do nothing!! You may set any string - draft_draft for example and test OK!

Comment 1

[Julien Wajsberg [:julienw]]

Definitely, thanks for your eagle eyes :)

Feel free to provide a patch !

Comment 2

[sv99]

Attachment: PR https://github.com/mozilla-b2g/gaia/pull/24161

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8491292 [details] [review]
PR https://github.com/mozilla-b2g/gaia/pull/24161

I'd prefer that you use the good old assert API, because all tests for SMS use it and for the sake of consistency I'd prefer that we use the same API throughout the tests.

Please request review again once you're ready :)

Thanks!

Comment 4

[sv99]

used in test-agent version chai - 0.5.3 not support assert.property(), available only expect interface!!

chai.js
654:  * # .property(name, [value])
 *
 * Assert that property of `name` exists, optionally with `value`.
 *
 *      var obj = { foo: 'bar' }
 *      expect(obj).to.have.property('foo');
 *      expect(obj).to.have.property('foo', 'bar');
 *      expect(obj).to.have.property('foo').to.be.a('string');
 *
 * @name property
 * @param {String} name
 * @param {*} value (optional)
 * @returns value of property for chaining
 * @api public
Assertion.prototype.property = function (name, val) {
  if (this.negate && undefined !== val) {
    if (undefined === this.obj[name]) {
      throw new Error(this.inspect + ' has no property ' + inspect(name));
    }
  } else {
    this.assert(
        undefined !== this.obj[name]
      , 'expected ' + this.inspect + ' to have a property ' + inspect(name)
      , 'expected ' + this.inspect + ' to not have property ' + inspect(name));
  }

  if (undefined !== val) {
    this.assert(
        val === this.obj[name]
      , 'expected ' + this.inspect + ' to have a property ' + inspect(name) + ' of ' +
          inspect(val) + ', but got ' + inspect(this.obj[name])
      , 'expected ' + this.inspect + ' to not have a property ' + inspect(name) + ' of ' +  inspect(val)
      , val
      , this.obj[val]
    );
  }

  this.obj = this.obj[name];
  return this;
};

Comment 5

[Julien Wajsberg [:julienw]]

Ah too bad ! We really need to update this so old version of chai eventually...

We could use "assert.ok(Thread.prototype.hasOwnProperty(drafts))" but it's not really better.

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8491292 [details] [review]
PR https://github.com/mozilla-b2g/gaia/pull/24161

Thanks, let's land this !
r=me

Comment 7

[Julien Wajsberg [:julienw]]

master: b4924cea332a01dd6daa3d92a661eabc9a43d971