Code review comment for lp://staging/~fs-8/selenium-simple-test/sst-remote-fixes

Revision history for this message
Corey Goldberg (coreygoldberg) wrote :

thanks for picking up that mistake.. you can see that sst.remote is neglected in terms of test coverage compared to the rest of sst :)

one small nitpick before merging this branch:
the way you have brackets/parens closed is not pep8 compliant, so it breaks our pep8 unit test.
can you fix up the closing brackets/parens and re-push to your branch?

it should be like:

    browser_factory = browsers.RemoteBrowserFactory(
        cmd_opts.webdriver_remote_url,
        {
            "browserName": cmd_opts.browser_type.lower(),
            "platform": cmd_opts.browser_platform.upper(),
            "version": cmd_opts.browser_version,
            "javascriptEnabled": not cmd_opts.javascript_disabled,
            "name": cmd_opts.session_name,
        },
    )

regards,
-Corey

review: Needs Fixing

« Back to merge proposal