998759 - Remove chai.js copying when running make update-common

Status: RESOLVED FIXED

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

Description

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Attachment: Gaia PR

Comment 2

[Julien Wajsberg [:julienw]]

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?

Comment 3

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 4

[Julien Wajsberg [:julienw]]

Note that "sandbox for each test" has already landed :)

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

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)

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Merged.

https://github.com/mozilla-b2g/gaia/commit/616adbea1e2b52d0399aa35312755a723e94674b

Comment 7

[Julien Wajsberg [:julienw]]

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?

Comment 8

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

yes we should use update-common, but I don't know why it named common-install, it shoud be named common-check or something...

Comment 9

[Julien Wajsberg [:julienw]]

because it used to run "npm install" in the test-agent directory, but this was removed around one month ago :)

Comment 10

[Julien Wajsberg [:julienw]]

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