Merge lp://qastaging/~coreygoldberg/selenium-simple-test/bmobproxy-har into lp://qastaging/selenium-simple-test

Command line parameter for browsermob should be --browsermob (--proxy is too generic for this specific functionality)

We *really* need a changelog now that we're doing proper releases.

Still reading the rest of the mp.

Similarly config.proxy_enabled should be renamed config.browsermob_enabled

I would marginally prefer "if proxy is not None:" to "if proxy:", purely as a stylistic issue. Not a big deal.

Line 324 - you can use "x.isalnum()" instead of "x.isalpha() or x.isdigit()".

When _make_useable_har_name is called without args - on clicking a link for example - then the generated file names will look like:

    har-date-.har

Because stem is empty. Is this ideal? Maybe omit the trailing slash for an empty stem, or use a non-empty stem for default instead of ''.

There's some duplicated code for saving har files (etc) in various actions. This could potentially be implemented as a decorator. *Unfortunately* using the standard decorator pattern clobbers function signatures - so our introspection code that generates the docs would need to be updated. (All decorated functions tend to have a *args, **kwargs signature - our decorator could expose the undecorated function as an attribute, and our doc generating code could know to check that attribute first for finding the function signature.)

Line 638: should this warning be an error and stop the test run? If not should cmd_opts.proxy be set to False so that we don't *attempt* to start browsermob.

just pushed a new version with changes...

> Command line parameter for browsermob should be
> --browsermob (--proxy is too generic for this
> specific functionality)

done.

> Similarly config.proxy_enabled should be renamed
> config.browsermob_enabled

done. all var names referring to proxy now more clearly named.

> I would marginally prefer "if proxy is not None:" to "if proxy:",
> purely as a stylistic issue. Not a big deal.

done. changed those also.

> Line 324 - you can use "x.isalnum()" instead of
> "x.isalpha() or x.isdigit()".

done. (nice tip)

> When _make_useable_har_name is called without args - on clicking a
> link for example - then the generated file names will look like:
> har-date-.har
> Because stem is empty. Is this ideal? Maybe omit the trailing slash

done. if no stem is provided, format will be har-date.har

> Line 638: should this warning be an error and stop the test run?
> If not should cmd_opts.proxy be set to False so that we don't
> *attempt* to start browsermob.

it's somewhat useful for development to run the internal tests with
browsermob on. it actually still routes requests through the proxy,
it just doesn't record them. i can change it to stop the test run, if
you think a warning will cause confusion.

> There's some duplicated code for saving har files (etc) in various
> actions. This could potentially be implemented as a decorator.

I was thinking a decorator could could be used, but wasn't totally
sure how to approach it. can discuss.

> We *really* need a changelog now that we're doing proper releases.

I will add a changelog.rst in a followup MP.

Ok, we can discuss the decorator approach - not sure its *necessary*, but it would remove the code duplication.

Note that the doc update still says "config.proxy_enabled" which should now be "config.browsermob_enabled". (Line 184 of diff.)

Other changes look good.

just pushed fix for doc.
changed "config.proxy_enabled" to "config.browsermob_enabled"

added changelog and pushed.

Great stuff. The check_flags action is new and doesn't appear in the changelog, but we can add that separately.