Closed Bug 1036926 Opened 5 years ago Closed 5 years ago

Emulator.setup_port_forwarding() should allow specification of a 'local_port'

Categories

(Testing :: Mozbase, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla33

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(1 file)

Currently it just returns a random open port. This is because this method only used to be used with marionette and marionette-client doesn't care which local port is chosen.

Now that this lives outside marionette, there might be other applications that *do* care (in my case marionette-js-runner).
This seems to do the trick. Unfortunately this is backwards incompatible again :(. If I reversed the variables (i.e remote_port, local_port) then it would be backwards compatible, but that ordering is counter-intuitive. Dave pegged marionette on all the old branches to mozrunner 5.37, so hopefully this time it won't be a big deal.

I have some other mozrunner changes I need to land at the same time, so I'll wait until that is in too before version bumping.
Attachment #8453747 - Flags: review?(wlachance)
Comment on attachment 8453747 [details] [diff] [review]
Add local_port variable to setup_port_forwarding

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

This makes sense to me, I'd like to get some feedback from Dave on whether this is likely to cause problems before landing
Attachment #8453747 - Flags: review?(wlachance)
Attachment #8453747 - Flags: review+
Attachment #8453747 - Flags: feedback?(dave.hunt)
Comment on attachment 8453747 [details] [diff] [review]
Add local_port variable to setup_port_forwarding

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

I can't foresee any issues, at least not from the Gaia UI tests perspective, as those are not currently using mozrunner for on-device testing.
Attachment #8453747 - Flags: feedback?(dave.hunt) → feedback+
(In reply to Dave Hunt (:davehunt) from comment #3)
> I can't foresee any issues, at least not from the Gaia UI tests perspective,
> as those are not currently using mozrunner for on-device testing.

Will there be any dependency issues though? This is another simultaneous change to mozrunner + marionette, so we'll need to version bump both of them at the same time.
The v1.3 and v1.4 branches of gaiatest are pinned to older mozrunner versions, so they should be fine. The master gaiatest is pinned for marionette, but marionette will pick up the latest mozrunner. If a simultaneous bump for marionette and mozrunner is required then we should at least have a version bump patch for the marionette client dependency in gaiatest ready, for when both are available on PyPI. That said, we don't run gaiatest on emulators, so I don't think we'd hit this code path.
Blocks: 1040789
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd5e53d2aabd

Thanks, I filed bug 1040789 to track bumping once this lands on m-c. Also this patch was tested on try as part of:
https://tbpl.mozilla.org/?tree=Try&rev=3f0aa29095e7&showall=1
https://hg.mozilla.org/mozilla-central/rev/cd5e53d2aabd
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.