Closed
Bug 1070845
Opened 10 years ago
Closed 10 years ago
[sharedtest] unit test refactoring for new version chai.assert api
Categories
(Firefox OS Graveyard :: Gaia, defect)
Firefox OS Graveyard
Gaia
Tracking
(b2g-v2.2 fixed)
RESOLVED
FIXED
2.1 S8 (7Nov)
Tracking | Status | |
---|---|---|
b2g-v2.2 | --- | fixed |
People
(Reporter: sv99, Assigned: sv99)
References
Details
Attachments
(1 file, 3 obsolete files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:35.0) Gecko/20100101 Firefox/35.0 Build ID: 20140920030206 Steps to reproduce: test_agent used very old version chai library version=0.5.3 in the current version=1.9.1 deepEqual take prototype into consideration
Attachment #8492911 -
Flags: review?(felash)
Comment 2•10 years ago
|
||
Comment on attachment 8492911 [details] [review] Link to PR Redirecting to David Flanagan and Anthony for your respective files. The goal here is to prepare unit tests before upgrading to a newer version for chai. But it looks like sv99 also fixed an issue in image_utils, maybe David you can have a closer look. Thanks in advance :)
Attachment #8492911 -
Flags: review?(felash)
Attachment #8492911 -
Flags: review?(dflanagan)
Attachment #8492911 -
Flags: review?(anthony)
Updated•10 years ago
|
Assignee: nobody → sv99
Updated•10 years ago
|
Attachment #8492911 -
Flags: review?(anthony) → review+
Comment 3•10 years ago
|
||
Comment on attachment 8492911 [details] [review] Link to PR See my comments on github. I think there are serious problems with the proposed changes to the test. (But I think there are also serious problems with the test in its current state.) Please do land the fix for image_utils.js. Doing that in a new bug would be best. The changes to the image_utils tests don't seem to involve deepEquals. Are they also necessary for the upgrade to the new version of chai? If not, you could do those in a separate bug, too.
Attachment #8492911 -
Flags: review?(dflanagan) → review-
Attachment #8498695 -
Flags: review?(anthony)
Attachment #8498695 -
Attachment mime type: text/plain → text/x-github-pull-request
Comment on attachment 8498695 [details] Link to PR (rebased) >https://github.com/mozilla-b2g/gaia/pull/24657
Attachment #8498695 -
Attachment is obsolete: true
Attachment #8498695 -
Flags: review?(anthony)
Attachment #8498734 -
Flags: review?(anthony)
Comment 7•10 years ago
|
||
Comment on attachment 8498734 [details] [review] Link to PR (rebased) r+ for the Dialer part. We still need a review from David though.
Attachment #8498734 -
Flags: review?(dflanagan)
Attachment #8498734 -
Flags: review?(anthony)
Attachment #8498734 -
Flags: review+
Comment 8•10 years ago
|
||
Comment on attachment 8498734 [details] [review] Link to PR (rebased) Removing the review request from David until the tests for image_utils are properly fixed. Thanks !
Attachment #8498734 -
Flags: review?(dflanagan) → review-
Updated•10 years ago
|
Attachment #8492911 -
Attachment is obsolete: true
Attachment #8498734 -
Flags: review- → review?(felash)
Comment 10•10 years ago
|
||
Comment on attachment 8498734 [details] [review] Link to PR (rebased) I'd like that we fix some other tests that are in the same file. If you don't have enough time I'm fine with filing a separate bug though.
Attachment #8498734 -
Flags: review?(felash) → review-
Assignee | ||
Comment 11•10 years ago
|
||
What do you think about real promise check after upgrade chai?
Comment 12•10 years ago
|
||
What do you mean? You mean wait that we upgrade chai before fixing this? I don't think we should wait but you can file a separate bug instead if you want.
Assignee | ||
Comment 13•10 years ago
|
||
I try rewrite some test, need check.
Attachment #8498734 -
Flags: review- → review?(felash)
Comment 14•10 years ago
|
||
Comment on attachment 8498734 [details] [review] Link to PR (rebased) Some more comments thanks a lot !
Attachment #8498734 -
Flags: review?(felash) → review-
Assignee | ||
Comment 15•10 years ago
|
||
Corrected
Comment 16•10 years ago
|
||
Comment on attachment 8498734 [details] [review] Link to PR (rebased) Ok, I think this is ready for a review from David now!
Attachment #8498734 -
Flags: review?(dflanagan)
Attachment #8498734 -
Flags: review-
Attachment #8498734 -
Flags: feedback+
Comment 17•10 years ago
|
||
Comment on attachment 8498734 [details] [review] Link to PR (rebased) I don't understand all of the Promise-based code, but I'm pretty sure that these tests are not doing what they are supposed to do because some of them test for an error message that is not sent.
Attachment #8498734 -
Flags: review?(dflanagan) → review-
Assignee | ||
Comment 18•10 years ago
|
||
What version mocha used in the test-agent? Mocha from 1.18.0 / 2014-03-13 add promise support
Comment 19•10 years ago
|
||
We use an older mocha, but some work is ongoing to upgrade it (see bug 874510).
Assignee | ||
Comment 20•10 years ago
|
||
Attachment #8498734 -
Attachment is obsolete: true
Attachment #8508876 -
Flags: review?(felash)
Comment 21•10 years ago
|
||
Comment on attachment 8508876 [details] [review] Link to PR Some comments on github. I think you missed or misunderstood some of my early comments in the other PR :) Thanks again
Attachment #8508876 -
Flags: review?(felash)
Comment 22•10 years ago
|
||
Comment on attachment 8508876 [details] [review] Link to PR Hey David, I think it's ok now, can you have another look? thanks !
Attachment #8508876 -
Flags: review?(dflanagan)
Comment 23•10 years ago
|
||
Comment on attachment 8508876 [details] [review] Link to PR Please remove the brittle test against the createObjectURL error message (see comment on github), but otherwise, this looks good to me. Thanks for sorting this out. It forced me to read up on promises
Attachment #8508876 -
Flags: review?(dflanagan) → review+
Assignee | ||
Comment 24•10 years ago
|
||
check -> ensureReject renamed assert.equal(err.mesage -> assert.equal(err.name, 'TypeError'); replaced Thanks
Comment 25•10 years ago
|
||
NI myself, I'll have a last check on the patch before landing :) Thanks to you both!
Flags: needinfo?(felash)
Comment 27•10 years ago
|
||
Master: https://github.com/mozilla-b2g/gaia/commit/192dba1c7aab9e7019b9a3f7562b48174ce4a3f5
Updated•10 years ago
|
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•