851292 - Mozprofile pacURL in permissions.py not formatted properly for mochitests

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Halberstadt [:ahal]]

This is a regression I caused a while back in https://github.com/mozilla/mozbase/commit/ddab19ab677652d23023c6f11752c86163037da3

The motivation for the above patch was that I needed to specify the exact same information twice in two different method calls in the same module when proxying for peptest. However it appears that with mochitest, the address being proxied isn't the primary location (as defined in server-locations.txt) like it is with peptest.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 - allow user specified proxy information

This patch caters to both the peptest and mochitest use cases. It will use the remote specified by the primary location by default, but also gives consumers a chance to pass in their own remote values if needed (like in the mochitest case).

Works with both peptest and mochitest locally.

Comment 2

[Jeff Hammel]

Comment on attachment 725121 [details] [diff] [review]
Patch 1.0 - allow user specified proxy information

I didn't do a thorough audit but I believe this is good.

+    def pac_prefs(self, user_proxy={}):

In general, I caution (and usually r-) passing mutable values as defaults.  It is actually okay IFF you never mutate them.  You don't here, so this is fine.  Still... If you wanted to fix (and suffer the extra line of code), feel free/encouraged.

Comment 3

[Andrew Halberstadt [:ahal]]

Pushed to master with the mutable fix:
https://github.com/mozilla/mozbase/commit/43f9510e3d58bfed32790c82a57edac5f928474d

Comment 4

[Andrew Halberstadt [:ahal]]

Caused bustage, need to update the tests: https://travis-ci.org/mozilla/mozbase/builds/5725257

Comment 5

[Andrew Halberstadt [:ahal]]

https://github.com/mozilla/mozbase/commit/0d240e02eb0c1149d4830091a7405cef029c8bec

The tests actually caught a corner case I had forgotten about. Hooray for tests!