Closed Bug 998759 Opened 6 years ago Closed 6 years ago

Remove chai.js copying when running make update-common


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

Not set


(b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed


(Reporter: rickychien, Assigned: rickychien)



(Keywords: regression)


(1 file)

46 bytes, text/x-github-pull-request
: review+
Details | Review
Every time I run "make update-common" to sync my test-agent chagnes will also copy chai.js from node_modules, but I think this should not happen. Actually, our test-agent app use old version chai and it will arise a lot of test fails if moving to new version.

It would be more convenient if we remove this.
Attached file Gaia PR
Attachment #8409422 - Flags: review?(yurenju.mozilla)
Note that I have bug 991663 to upgrade mocha/chai/etc. In the future we should have the good one but you're right that some tests are failing with it :(

How comes that it doesn't fail on Travis?
Really? Upgrade chai even pass on Travis? hmm... sounds weird
I believe adding sandbox for each test file will fix local and Travis inconsistent issue.
So I don't have any idea about this.
Note that "sandbox for each test" has already landed :)
Comment on attachment 8409422 [details] [review]
Gaia PR

r=yurenju because using same version of chai between test-agent dependency and app unit test is not necessary. (but it would be good if we upgrade our chai version on another bug)
Attachment #8409422 - Flags: review?(yurenju.mozilla) → review+
Closed: 6 years ago
Resolution: --- → FIXED
And I know why it doesn't fail on travis: travis runs "make common-install", but not "make update-common". Should we make it run "make update-common" instead ? "common-install" is practically a noop these days... What do you think Yuren?
Flags: needinfo?(yurenju.mozilla)
yes we should use update-common, but I don't know why it named common-install, it shoud be named common-check or something...
Flags: needinfo?(yurenju.mozilla)
because it used to run "npm install" in the test-agent directory, but this was removed around one month ago :)
This was a regression from bug 976152 which is in v1.4 so I uplifted this to v1.4 as well with a=tests.

v1.4: 62ec84411ecf94dcb80921631e04bc7db25625bb
Blocks: 976152
Keywords: regression
You need to log in before you can comment on or make changes to this bug.