899725 - the test-agent should sort the test files before running them

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Julien Wajsberg [:julienw]]

Currently, the order which the test files are run in is environment-dependent, which can make some failures hidden in Travis while apparent on some developer environments.

It would be a nice idea to always sort the test files so that they're always run in the same order.

Comment 1

[James Lal [:lightsofapollo]]

IIRC the code is here: https://github.com/mozilla-b2g/js-test-agent/blob/master/lib/node/bin/test.js#L7 if you add a .sort() to that it should do what you expect (for all of test agent).

Comment 2

[Julien Wajsberg [:julienw]]

James, I'd prefer to sort here:

https://github.com/mozilla-b2g/js-test-agent/blob/master/test-agent.js#L2941

what do you think ?

If we do it in tests.js we'll sort only the files that are passed as arguments, but not when we run all tests.

Comment 3

[Julien Wajsberg [:julienw]]

Attachment: patch v1

see also PR at https://github.com/mozilla-b2g/js-test-agent/pull/37

Comment 4

[Julien Wajsberg [:julienw]]

So to use this merged PR we need to do a new release of the test-agent and bump up the dependencies, right ?

Comment 5

[James Lal [:lightsofapollo]]

Correct- I will push a new version out.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

To test this patch locally, which version of the test-agent do we want to apply it to.  On my machine I see:

  ./test_apps/test-agent/common/vendor/test-agent/test-agent.js
  ./tools/test-agent/node_modules/test-agent/test-agent.js
  ./tools/test-agent/node_modules/b2g-scripts/node_modules/marionette-client/vendor  /test-agent.js
  ./tools/test-agent/node_modules/b2g-scripts/node_modules/marionette-client/node_modules/test-agent/test-agent.js
  ./tools/test-agent/node_modules/b2g-scripts/node_modules/test-agent/test-agent.js

The one in test_apps seems to be the one I want, but thought I would ask.

Comment 7

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/7eecfb522e3f4ea1a38962b375013267138a51b6

This did not fix the current unit test issues but it didn't get any worse either.

Comment 8

[Julien Wajsberg [:julienw]]

If I'm not wrong, you forgot to call "make update-common" and therefore the changes are not in the test-agent test_apps.

Or should we call the "update-common" target as dependency of "make test-agent-server" ? (it's currently not done).

Comment 9

[James Lal [:lightsofapollo]]

So because we run make update-common before running travis the changes are present but not in the git history of gaia =/ Sorry! I should have remembered this step and will land this in gaia proper now.

Comment 10

[Julien Wajsberg [:julienw]]

(In reply to James Lal [:lightsofapollo] from comment #9)
> So because we run make update-common before running travis the changes are
> present

but not for human users then, right ? :)

So my question still applies: don't you think we should call the "update-common" target as a dependency of "make test-agent-server" ?

> but not in the git history of gaia =/ Sorry! I should have
> remembered this step and will land this in gaia proper now.

have you done it ? :)

Comment 11

[James Lal [:lightsofapollo]]

yes (damn your up early) https://github.com/mozilla-b2g/gaia/commit/ddf78455bbfcc2a2adba2e620443c19cbb325208

Comment 12

[James Lal [:lightsofapollo]]

In general update-common should only be run by the people landing code... In reality the travis CI script runs it anyway so it happened to fix this issue. 

Ultimately we should just get rid of all copy/paste operations when we need node anyway for the good stuff (my preference).

Comment 13

[Julien Wajsberg [:julienw]]

(damn you're up late :p)

Thanks for the landing, all this was to make all environment behave the same, so I really wanted to have developers run the same code as travis :)

I agree for the copy/paste operations, right now it's quite challenging to know which test-agent file is run by which program :)

Comment 14

[Julien Wajsberg [:julienw]]

Comment on attachment 791719 [details] [diff] [review]
patch v1

this was merged in the js-test-agent repository.