861295 - Modify --testing-modules-dir for android xpcshell tests

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Geoff Brown [:gbrown]]

In bug 828069, I asked :Callek to pass --testing-modules-dir to remotexpcshelltests.py, and he implemented that accurately.

But from the discussion in bug 857252, I'm pretty sure I was completely wrong: we should not pass --testing-modules-dir at all and should instead rely on the hack at https://hg.mozilla.org/mozilla-central/file/7712e67a7c1e/testing/xpcshell/runxpcshelltests.py#l690.

It looks to me like --testing-modules-dir is not passed when running the desktop xpcshell tests.

Using --testing-modules-dir for android xpcshell tests does not cause any problems currently, but causes various failures when trying to run additional tests: https://tbpl.mozilla.org/?tree=Try&rev=9581bc27560a

(See bug 762837 for the original work on testing-modules-dir.)

Comment 1

[Geoff Brown [:gbrown]]

It turns out that if --testing-modules-dir is not passed to remotexpcshelltests.py, the hack in runxpcshelltests.py is not effective: https://tbpl.mozilla.org/?tree=Try&rev=6b0dea0f831e.

Basically the hack sets the modules directory too late for the remote script to do the correct initialization/setup.

Moving the hack such that it is effective for the remote case looks complicated.

I could duplicate the hack in remotexpcshelltests.py, but I wonder if it wouldn't be better if we simply pass the correct modules directory on the command line. This try run verifies that --testing-modules-dir=xpcshell/../modules works fine: https://tbpl.mozilla.org/?tree=Try&rev=b57b9152dce9. 

I think "--testing-modules-dir=modules" ought to work fine.

Comment 2

[Justin Wood (:Callek)]

Attachment: [custom] use modules/ instead

This should do it

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 743149 [details] [diff] [review]
[custom] use modules/ instead

Will this somehow affect other branches?

Comment 4

[Justin Wood (:Callek)]

(In reply to Rail Aliiev [:rail] from comment #3)
> Comment on attachment 743149 [details] [diff] [review]
> [custom] use modules/ instead
> 
> Will this somehow affect other branches?

No, its only active on trunk/related. It was a newly added test suite. And this change won't break/hurt any test that is already running this anyway (we only enabled one test so far, where the modules/ is not technically required, this change allows us to enable more tests)

Comment 5

[Geoff Brown [:gbrown]]

Comment on attachment 743149 [details] [diff] [review]
[custom] use modules/ instead

Review of attachment 743149 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 743149 [details] [diff] [review]
[custom] use modules/ instead

https://hg.mozilla.org/build/buildbotcustom/rev/08dcd33abccb

Comment 7

[Rail Aliiev [:rail]]

In production