956648 - Use 10s default unit test timeout

Status: RESOLVED FIXED

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

Description

[Kevin Grandon :kgrandon]

Travis generally runs much slower than testing on a local machine. Because IndexedDB access may take a long time, and often blows past the default 2s timeout, we should use a default timeout of 20s.

Comment 1

[Kevin Grandon :kgrandon]

Once this is done, I think we can remove all of the this.timeout() calls scattered throughout the tests.

Comment 2

[Julien Wajsberg [:julienw]]

In an ideal world, I'd rather have the 20 second timeouts in IndexedDB tests only, but in our world where developers are still doing asynchronous tests, I agree with this bug to lower the intermittent frequency.

Comment 3

[Kevin Grandon :kgrandon]

Julien, James - Before I dig into test-agent code, would you happen to know off the top of your head the best way to implement this or where to look? If you want to write a quick patch for this too, that would be nice :)  Thanks!

Comment 4

[Kevin Grandon :kgrandon]

Attachment: Github pull request

This might do what I want to do - but I'm not entirely sure the best way to test it until we run on travis.

Comment 5

[Julien Wajsberg [:julienw]]

I think this is the correct way.

Usually, to code for test-agent, I modify directly the file test-agent.js in gaia's test_apps, and then, once it works as I want, I report the changes in js-test-agent. I'm sure there is a better way to work with npm ;-)

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8356444 [details] [review]
Github pull request

adding review, I'll test today

Comment 7

[Kevin Grandon :kgrandon]

(In reply to Julien Wajsberg [:julienw] from comment #5)
> I think this is the correct way.
> 
> Usually, to code for test-agent, I modify directly the file test-agent.js in
> gaia's test_apps, and then, once it works as I want, I report the changes in
> js-test-agent. I'm sure there is a better way to work with npm ;-)

Yeah, that sounds like a good way. I guess for testing on travis we should ask for npm permissions, then we could publish and test with an alpha version.

Comment 8

[Julien Wajsberg [:julienw]]

I wonder if there is a better way to work locally with npm, with the local .npm cache. Like publishing to ~/.npm an alpha version. Coming from Java I know it's possible with maven.

Comment 9

[Kevin Grandon :kgrandon]

(In reply to Julien Wajsberg [:julienw] from comment #8)
> I wonder if there is a better way to work locally with npm, with the local
> .npm cache. Like publishing to ~/.npm an alpha version. Coming from Java I
> know it's possible with maven.

For working locally with npm you can use the npm link command. Maybe you are looking for that?

https://npmjs.org/doc/cli/npm-link.html

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 8356444 [details] [review]
Github pull request

r=me but with 10 seconds instead of 20 seconds.

Comment 11

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/js-test-agent/commit/b72866eb516410df29c97ba4d64a2c22500cf3de

Comment 12

[Kevin Grandon :kgrandon]

Attachment: Pull Request - Update test-agent version

Comment 13

[Julien Wajsberg [:julienw]]

js-test-agent master commit: 810342e8b2043cfeb8a66553161ec6a8db73fc22
gaia master commit: a9b0870224c3edcfbba2c276c05b47812610b61d

Kevin, you need to run "make update-common" and do a PR from this too. For the test-agent, we don't "npm install" automatically (yet).

Comment 14

[Kevin Grandon :kgrandon]

Attachment: Github pull request - Remove test timeouts, update js agent

Hi Julien - Thanks for the info. I have ~3 green test runs after updating the test agent and removing timeouts. I think we should be good to go here, can you review this one?

Comment 15

[Julien Wajsberg [:julienw]]

Comment on attachment 8363093 [details] [review]
Github pull request - Remove test timeouts, update js agent

r=me

you missed some timeout calls in the "communications/" apps, and I think we should keep the bigger timeouts in some of the files (although I can accept that we remove them now and we add them back if we need to, so you can land with all timeout calls removed if you want).

Comment 16

[Kevin Grandon :kgrandon]

I added the calendar ones here, and I think that 10s is acceptable. I would like to land with them all removed if possible, and if there are problems, we can re-investigate. Thanks for the review!

Comment 17

[Julien Wajsberg [:julienw]]

Good for me, let's do this.

Comment 18

[Kevin Grandon :kgrandon]

Landed after 2 green runs: https://github.com/mozilla-b2g/gaia/commit/2162b4a2fb2cb08d93433c574462062286cd3f05

I'm going to keep monitoring travis to ensure we don't have intermittent because of this. This should solve the last few intermittent failures that we have been seeing recently.