Closed Bug 1312739 Opened 8 years ago Closed 7 years ago

Passing relative path doesn't work for browser-chrome mochitests on interactive loaner

Categories

(Testing :: General, defect)

Product:
Testing
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: stas, Assigned: ahal)

References

Details

Attachments

(3 files)

Bug 1312739 - Fix marionette SEARCH_PATHS in mach_test_package_bootstrap.py,
59 bytes, text/x-review-board-request
jmaher
: review+
Details
Bug 1312739 - Move mochitest 'ALL_FLAVORS' dict from mach_commands.py to mochitest_options.py,
59 bytes, text/x-review-board-request
jmaher
: review+
Details
Bug 1312739 - Use ALL_FLAVORS dict when running mochitest from an interactive loaner,
59 bytes, text/x-review-board-request
jmaher
: review+
Details 
I first thought this was related to bug 1311723 but the problem persists even with mozinfo.json.

After clicking "One-click loaner", I choose the ssh session and select 2 from the wizard menu.  I then issue:

$ mach mochitest workspace/build/tests/mochitest/browser/browser/base/content/test/general/browser_page_style_menu.js

Checking for orphan ssltunnel processes...
Checking for orphan xpcshell processes...
0 ERROR no tests to run using specified combination of filters: skip_if, run_if, fail_if, remove_imptest_failure_expectations, subsuite(name=None), pathprefix(['/
home/worker/workspace/build/tests/mochitest/browser/browser/base/content/test/general/browser_page_style_menu.js'])
SUITE-START | Running 0 tests
1 ERROR no tests to run using specified combination of filters: skip_if, run_if, fail_if, remove_imptest_failure_expectations, subsuite(name=None), pathprefix(['/
home/worker/workspace/build/tests/mochitest/browser/browser/base/content/test/general/browser_page_style_menu.js'])
0 INFO TEST-START | Shutdown
1 INFO Passed:  0
2 INFO Failed:  0
3 INFO Todo:    0
4 INFO Mode:    non-e10s

Any ideas what might be causing it?
I was able to get this to work:
mach mochitest -f browser browser/browser/base/content/test/general/browser_page_style_menu.js

But there are definitely issues to sort out here. Passing in the full path like you did should work, as should passing in the path relative to $topsrcdir (which also doesn't work). It should also probably detect the flavor automatically.

Not sure about you, but the terminal is also acting wonky for me when the command exceeds a certain length. That's not something I know how to fix, but I'll file a bug.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
(In reply to Andrew Halberstadt [:ahal] from comment #1)
> I was able to get this to work:
> mach mochitest -f browser
> browser/browser/base/content/test/general/browser_page_style_menu.js

A-ha!  Great, that's what I needed!

> But there are definitely issues to sort out here. Passing in the full path
> like you did should work, as should passing in the path relative to
> $topsrcdir (which also doesn't work). It should also probably detect the
> flavor automatically.

I think the most confusing part for me is the question whether it takes a path relative to the PWD or relative to some kind of source/build/dist/etc root.

> Not sure about you, but the terminal is also acting wonky for me when the
> command exceeds a certain length. That's not something I know how to fix,
> but I'll file a bug.

Yes, I'm seeing this on Firefox.  Chrome works fine -- just checked.  I think running `reset` can help sometimes but I haven't had success with it today.

Thanks again for looking into this!
Sorry this slipped, I'll try to take a look at it at some point soon. I think there are two issues to sort out:
1) We aren't automatically setting -f browser
2) The relative path only works with an extra 'browser' prepended (this is how these tests are structured in the tests.zip)
Summary: Unable to run a single mochitest on an interactive loaner → Passing relative path doesn't work for browser-chrome mochitests on interactive loaner
The above patches only fix problem 2) of comment 3. Automatically determining the flavor will be very tricky. When running from a local checkout, we use the build system to do this, but we don't have that luxury from a loaner. I have some patches that touch on test resolving that could make auto-determining the flavor easier, but I'd still be tempted to wait until we are running tests from the srcdir before tackling it.

For now with these patches, you'd run e.g:
mach mochitest -f browser browser/base/content
Comment on attachment 8820771 [details]
Bug 1312739 - Fix marionette SEARCH_PATHS in mach_test_package_bootstrap.py,

https://reviewboard.mozilla.org/r/100200/#review100772
Attachment #8820771 - Flags: review?(jmaher) → review+
Comment on attachment 8820772 [details]
Bug 1312739 - Move mochitest 'ALL_FLAVORS' dict from mach_commands.py to mochitest_options.py,

https://reviewboard.mozilla.org/r/100202/#review100776

::: testing/mochitest/mach_commands.py:280
(Diff revision 1)
> -    @CommandArgument('-f', '--flavor',
> -                     metavar='{{{}}}'.format(', '.join(CANONICAL_FLAVORS)),
> -                     choices=SUPPORTED_FLAVORS,
> -                     help='Only run tests of this flavor.')
>      def run_mochitest_general(self, flavor=None, test_objects=None, resolve_tests=True, **kwargs):
> +        from mochitest.mochitest_options import ALL_FLAVORS

as this is in the testing/mochitest/ direcotry, could we just do the import at the top of the file?  I personally am not a fan of imports insde a method, maybe that is my roots in C programming.
Attachment #8820772 - Flags: review?(jmaher) → review-
Comment on attachment 8820773 [details]
Bug 1312739 - Use ALL_FLAVORS dict when running mochitest from an interactive loaner,

https://reviewboard.mozilla.org/r/100204/#review100780

this looks good!
Attachment #8820773 - Flags: review?(jmaher) → review+
Comment on attachment 8820772 [details]
Bug 1312739 - Move mochitest 'ALL_FLAVORS' dict from mach_commands.py to mochitest_options.py,

https://reviewboard.mozilla.org/r/100202/#review100776

> as this is in the testing/mochitest/ direcotry, could we just do the import at the top of the file?  I personally am not a fan of imports insde a method, maybe that is my roots in C programming.

That is normally a good point, except for in mach_commands.py.

The reason is because all mach_commands.py are loaded every single time the 'mach' binary is run, which means anything a mach_commands.py imports globally is also loaded. For that reason, global imports in mach_commands.py are actually discouraged (contrary to standard python practice). If I made this a global import, that means that we'd be loading mochitest_options.py even if you ran, e.g |mach help|. While a single import doesn't make a huge perf difference, it starts to add up if we do it too often.
Comment on attachment 8820772 [details]
Bug 1312739 - Move mochitest 'ALL_FLAVORS' dict from mach_commands.py to mochitest_options.py,

https://reviewboard.mozilla.org/r/100202/#review100822

thanks for the explanation
Attachment #8820772 - Flags: review- → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2adce340421
Fix marionette SEARCH_PATHS in mach_test_package_bootstrap.py, r=jmaher
https://hg.mozilla.org/integration/autoland/rev/ecb1d15e8075
Move mochitest 'ALL_FLAVORS' dict from mach_commands.py to mochitest_options.py, r=jmaher
https://hg.mozilla.org/integration/autoland/rev/1f3f88337227
Use ALL_FLAVORS dict when running mochitest from an interactive loaner, r=jmaher
I'll have to take a look after the break. I had tested this on try, but only for Linux opt, which of course was the only platform/buildtype that *didn't* fail:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a9068c5c6649453ebc29127fc804b3f90104b4d
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50dc2c5fe0f1
Fix marionette SEARCH_PATHS in mach_test_package_bootstrap.py, r=jmaher
https://hg.mozilla.org/integration/autoland/rev/7dbc513c553f
Move mochitest 'ALL_FLAVORS' dict from mach_commands.py to mochitest_options.py, r=jmaher
https://hg.mozilla.org/integration/autoland/rev/b5af607d2990
Use ALL_FLAVORS dict when running mochitest from an interactive loaner, r=jmaher
https://hg.mozilla.org/mozilla-central/rev/50dc2c5fe0f1
https://hg.mozilla.org/mozilla-central/rev/7dbc513c553f
https://hg.mozilla.org/mozilla-central/rev/b5af607d2990
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1328549
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: