If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: julienw, Assigned: julienw)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
Flags: needinfo?(jlal)
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).
Flags: needinfo?(jlal)
(Assignee)

Comment 2

4 years ago
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.
Flags: needinfo?(jlal)
(Assignee)

Comment 3

4 years ago
Created attachment 791719 [details] [diff] [review]
patch v1

see also PR at https://github.com/mozilla-b2g/js-test-agent/pull/37
Assignee: nobody → felash
Attachment #791719 - Flags: review?(jlal)
(Assignee)

Comment 4

4 years ago
So to use this merged PR we need to do a new release of the test-agent and bump up the dependencies, right ?
Correct- I will push a new version out.
Flags: needinfo?(jlal)

Comment 6

4 years ago
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.

Updated

4 years ago
Blocks: 906261

Updated

4 years ago
Blocks: 906203
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.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

4 years ago
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).
Flags: needinfo?(jlal)
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.
Flags: needinfo?(jlal)
(Assignee)

Comment 10

4 years ago
(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 ? :)
yes (damn your up early) https://github.com/mozilla-b2g/gaia/commit/ddf78455bbfcc2a2adba2e620443c19cbb325208
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).
(Assignee)

Comment 13

4 years ago
(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 :)
(Assignee)

Comment 14

4 years ago
Comment on attachment 791719 [details] [diff] [review]
patch v1

this was merged in the js-test-agent repository.
Attachment #791719 - Flags: review?(jlal)
You need to log in before you can comment on or make changes to this bug.