Closed
Bug 1470659
Opened 6 years ago
Closed 6 years ago
Add setWindowRect capability
Categories
(Remote Protocol :: Marionette, enhancement, P1)
Remote Protocol
Marionette
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: ato, Assigned: ato)
References
(Blocks 1 open bug)
Details
Attachments
(7 files, 21 obsolete files)
3.08 KB,
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
8.88 KB,
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
10.63 KB,
patch
|
automatedtester
:
review+
|
Details | Diff | Splinter Review |
6.58 KB,
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
2.64 KB,
patch
|
automatedtester
:
review+
whimboo
:
review+
|
Details | Diff | Splinter Review |
4.37 KB,
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
4.72 KB,
patch
|
ato
:
review+
|
Details | Diff | Splinter Review |
Marionette is missing the setWindowRect capability mandated by WebDriver.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
Change the switch-statement to only do assertions, and replace the v variable with any parsed values before writing it to the matched set of capabilities in one location.
Assignee | ||
Comment 2•6 years ago
|
||
The setWindowRect capability is mandated by the WebDriver standard and is an indication whether the driver supports manipulating the window dimensions and position. This will always be true for Firefox and always false for Fennec.
Assignee | ||
Comment 3•6 years ago
|
||
It will never be possible to configure setWindowRect, and trying to do so will cause geckodriver to return with an error.
Assignee | ||
Comment 4•6 years ago
|
||
Specific tests for the platformName capability do not belong in the test for the response body structure.
Assignee | ||
Comment 5•6 years ago
|
||
For similar reasons as for platformName, tests for configuring the timeouts object do not belong in the same parent test as those for response body structure.
Assignee | ||
Comment 6•6 years ago
|
||
To be able to ignore tests for individual capabilities we need to parametrize these tests. geckodriver now supports setWindowRect, but fails the proxy capability test because it is for some reason not propagated back.
Assignee | ||
Comment 7•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #8993935 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993936 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993937 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993938 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993939 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993940 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993941 -
Flags: review+
Assignee | ||
Comment 8•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=074a58643fa8e710e0de3b6b6a111110f88fc038
Assignee | ||
Comment 9•6 years ago
|
||
Change the switch-statement to only do assertions, and replace the v variable with any parsed values before writing it to the matched set of capabilities in one location.
Attachment #8994009 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993935 -
Attachment is obsolete: true
Attachment #8993935 -
Flags: review?(dburns)
Assignee | ||
Comment 10•6 years ago
|
||
The setWindowRect capability is mandated by the WebDriver standard and is an indication whether the driver supports manipulating the window dimensions and position. This will always be true for Firefox and always false for Fennec.
Attachment #8994010 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993936 -
Attachment is obsolete: true
Attachment #8993936 -
Flags: review?(dburns)
Assignee | ||
Comment 11•6 years ago
|
||
It will never be possible to configure setWindowRect, and trying to do so will cause geckodriver to return with an error.
Attachment #8993937 -
Attachment is obsolete: true
Attachment #8994011 -
Flags: review?(dburns)
Attachment #8993937 -
Flags: review?(dburns)
Assignee | ||
Comment 12•6 years ago
|
||
Specific tests for the platformName capability do not belong in the test for the response body structure.
Attachment #8993938 -
Attachment is obsolete: true
Attachment #8994012 -
Flags: review?(dburns)
Attachment #8993938 -
Flags: review?(dburns)
Assignee | ||
Comment 13•6 years ago
|
||
For similar reasons as for platformName, tests for configuring the timeouts object do not belong in the same parent test as those for response body structure.
Attachment #8994013 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993939 -
Attachment is obsolete: true
Attachment #8993939 -
Flags: review?(dburns)
Assignee | ||
Comment 14•6 years ago
|
||
To be able to ignore tests for individual capabilities we need to parametrize these tests. geckodriver now supports setWindowRect, but fails the proxy capability test because it is for some reason not propagated back.
Attachment #8994014 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8993940 -
Attachment is obsolete: true
Attachment #8993940 -
Flags: review?(dburns)
Assignee | ||
Comment 15•6 years ago
|
||
Attachment #8993941 -
Attachment is obsolete: true
Attachment #8994015 -
Flags: review+
Assignee | ||
Comment 16•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba022f2f59c1a505cf61fabc7f4a711ebc3d183c
Comment 17•6 years ago
|
||
Comment on attachment 8994009 [details] [diff] [review] Simplify Marionette capability parsing. Review of attachment 8994009 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/marionette/capabilities.js @@ +475,3 @@ > > if (Object.values(PageLoadStrategy).includes(v)) { > matched.set("pageLoadStrategy", v); I think we want to remove this call to `set` too, by negating the check and replace it with the else case, right? @@ +475,5 @@ > > if (Object.values(PageLoadStrategy).includes(v)) { > matched.set("pageLoadStrategy", v); > } else { > + throw new InvalidArgumentError("Unknown page load strategy: " + v); Use a template string here to not have to use `+`. @@ +488,1 @@ > break; Remove the extra `break`. @@ +494,5 @@ > case "unhandledPromptBehavior": > assert.string(v, > pprint`Expected ${k} to be a string, got ${v}`); > > if (Object.values(UnhandledPromptBehavior).includes(v)) { Same as for page load strategy.
Attachment #8994009 -
Flags: review-
Comment 18•6 years ago
|
||
Comment on attachment 8994010 [details] [diff] [review] Add setWindowRect capability to Marionette. Review of attachment 8994010 [details] [diff] [review]: ----------------------------------------------------------------- Please also add Python unit tests for this addition in test_capabilities.py. ::: testing/marionette/capabilities.js @@ +488,5 @@ > break; > + > + case "setWindowRect": > + assert.boolean(v, pprint`Expected ${k} to be boolean, got ${v}`); > + if (v && Services.appinfo.name != "Firefox") { Why don't you use the global `appinfo` object here in regards of `firefox` vs. `Firefox`?
Attachment #8994010 -
Flags: review-
Comment 19•6 years ago
|
||
Comment on attachment 8994012 [details] [diff] [review] Move platformName test to separate file. Review of attachment 8994012 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/new_session/merge.py @@ +7,3 @@ > > > +@pytest.mark.skipif(platform_name is None, reason="Unsupported platform") Mind to add the platform name to the reason string? ::: testing/web-platform/tests/webdriver/tests/new_session/page_load_strategy.py @@ +3,5 @@ > +def test_pageLoadStrategy(new_session, add_browser_capabilities): > + response, _ = new_session({"capabilities": { > + "alwaysMatch": add_browser_capabilities({"pageLoadStrategy": "eager"})}}) > + value = assert_success(response) > + nit: you can remove this empty line. ::: testing/web-platform/tests/webdriver/tests/new_session/platform_name.py @@ +1,2 @@ > +import pytest > +import sys Unused import of sys. @@ +1,5 @@ > +import pytest > +import sys > + > +from tests.support.asserts import assert_success > +from tests.support import platform_name nit: those lines should be flipped. @@ +4,5 @@ > +from tests.support.asserts import assert_success > +from tests.support import platform_name > + > + > +@pytest.mark.skip_if(platform_name is None) Please add a reason string similar to the merge.py file. ::: testing/web-platform/tests/webdriver/tests/support/capabilities.py @@ +1,1 @@ > +platform_name = { I don't see this file being used anywhere in favor of __init__.py. So please remove.
Attachment #8994012 -
Flags: review-
Updated•6 years ago
|
Attachment #8994013 -
Flags: review+
Comment 20•6 years ago
|
||
Comment on attachment 8994014 [details] [diff] [review] Parametrize capabilities tests. Review of attachment 8994014 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/new_session/response.py @@ +1,3 @@ > # META: timeout=long > > +import pytest pytest is not a standard lib module and needs to be added after `uuid`. @@ +20,5 @@ > + ("acceptInsecureCerts", bool), > + ("setWindowRect", bool), > + ("timeouts", dict), > + ("proxy", dict), > + ("pageLoadStrategy", basestring), Mind sorting the caps as listed in the spec? It would make it easier to compare if something is missing. @@ +22,5 @@ > + ("timeouts", dict), > + ("proxy", dict), > + ("pageLoadStrategy", basestring), > +]) > +def test_capability_type(new_session, add_browser_capabilities, capability, type): Please note that this parametrization will unnecessarily restart the browser for each parameter. Given that we use no browser capabilities you want to use the `session` fixture instead. @@ +35,5 @@ > + ("acceptInsecureCerts", False), > + ("setWindowRect", True), > + ("timeouts", {"implicit": 0, "pageLoad": 300000, "script": 30000}), > + ("proxy", {}), > + ("pageLoadStrategy", "normal"), Looks like I missed to add the test for the `unhandledPromptBehavior` capability. Mind adding it here too? @@ +42,1 @@ > response, _ = new_session({"capabilities": {"alwaysMatch": add_browser_capabilities({})}}) Same problem for restarts as noted above.
Attachment #8994014 -
Flags: review-
Comment 21•6 years ago
|
||
Comment on attachment 8994014 [details] [diff] [review] Parametrize capabilities tests. Review of attachment 8994014 [details] [diff] [review]: ----------------------------------------------------------------- Agree with whimboo's comments, will let him finish this review :)
Attachment #8994014 -
Flags: review?(dburns)
Updated•6 years ago
|
Attachment #8994013 -
Flags: review?(dburns) → review+
Comment 22•6 years ago
|
||
Comment on attachment 8994012 [details] [diff] [review] Move platformName test to separate file. Review of attachment 8994012 [details] [diff] [review]: ----------------------------------------------------------------- Removing myself as reviewer as I agree with Whimboo's comments. I will let him follow up
Attachment #8994012 -
Flags: review?(dburns)
Updated•6 years ago
|
Attachment #8994011 -
Flags: review?(dburns) → review+
Comment 23•6 years ago
|
||
Comment on attachment 8994010 [details] [diff] [review] Add setWindowRect capability to Marionette. Review of attachment 8994010 [details] [diff] [review]: ----------------------------------------------------------------- I will let whimboo follow up here
Attachment #8994010 -
Flags: review?(dburns)
Comment 24•6 years ago
|
||
Comment on attachment 8994009 [details] [diff] [review] Simplify Marionette capability parsing. Review of attachment 8994009 [details] [diff] [review]: ----------------------------------------------------------------- I will let whimboo follow up here
Attachment #8994009 -
Flags: review?(dburns)
Assignee | ||
Comment 25•6 years ago
|
||
Change the switch-statement to only do assertions, and replace the v variable with any parsed values before writing it to the matched set of capabilities in one location.
Attachment #8995949 -
Flags: review?(hskupin)
Assignee | ||
Updated•6 years ago
|
Attachment #8994009 -
Attachment is obsolete: true
Assignee | ||
Comment 26•6 years ago
|
||
The setWindowRect capability is mandated by the WebDriver standard and is an indication whether the driver supports manipulating the window dimensions and position. This will always be true for Firefox and always false for Fennec.
Attachment #8994010 -
Attachment is obsolete: true
Attachment #8995950 -
Flags: review?(hskupin)
Assignee | ||
Comment 27•6 years ago
|
||
It will never be possible to configure setWindowRect, and trying to do so will cause geckodriver to return with an error.
Attachment #8995951 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8994011 -
Attachment is obsolete: true
Assignee | ||
Comment 28•6 years ago
|
||
Specific tests for the platformName capability do not belong in the test for the response body structure.
Attachment #8994012 -
Attachment is obsolete: true
Attachment #8995952 -
Flags: review?(hskupin)
Assignee | ||
Comment 29•6 years ago
|
||
For similar reasons as for platformName, tests for configuring the timeouts object do not belong in the same parent test as those for response body structure.
Attachment #8995953 -
Flags: review?(hskupin)
Attachment #8995953 -
Flags: review?(dburns)
Assignee | ||
Comment 30•6 years ago
|
||
To be able to ignore tests for individual capabilities we need to parametrize these tests. geckodriver now supports setWindowRect, but fails the proxy capability test because it is for some reason not propagated back.
Attachment #8995954 -
Flags: review?(hskupin)
Assignee | ||
Updated•6 years ago
|
Attachment #8994014 -
Attachment is obsolete: true
Assignee | ||
Comment 31•6 years ago
|
||
Attachment #8995955 -
Flags: review+
Assignee | ||
Updated•6 years ago
|
Attachment #8994015 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8995951 -
Flags: review?(dburns) → review+
Assignee | ||
Updated•6 years ago
|
Attachment #8995953 -
Flags: review?(hskupin)
Attachment #8995953 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8995951 -
Flags: review+
Updated•6 years ago
|
Attachment #8995949 -
Flags: review?(hskupin) → review+
Comment 32•6 years ago
|
||
Comment on attachment 8995950 [details] [diff] [review] Add setWindowRect capability to Marionette. Review of attachment 8995950 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py @@ +154,5 @@ > with self.assertRaisesRegexp(SessionNotCreatedException, "InvalidArgumentError"): > self.marionette.start_session({"pageLoadStrategy": value}) > > + def test_set_window_rect(self): > + if self.browser_name == "Fennec": Fennec won't exist that long anymore, so better to put it into the else case instead of Firefox.
Attachment #8995950 -
Flags: review?(hskupin) → review+
Comment 33•6 years ago
|
||
Comment on attachment 8995952 [details] [diff] [review] Move platformName test to separate file. Review of attachment 8995952 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/new_session/platform_name.py @@ +1,4 @@ > +import pytest > + > +from tests.support.asserts import assert_success > +from tests.support import platform_name why not flipped? @@ +3,5 @@ > +from tests.support.asserts import assert_success > +from tests.support import platform_name > + > + > +@pytest.mark.skip_if(platform_name is None, reason="Unsupported platform %s" % platform_name) Lets prefer to use `format` all over the place. @@ +4,5 @@ > +from tests.support import platform_name > + > + > +@pytest.mark.skip_if(platform_name is None, reason="Unsupported platform %s" % platform_name) > +def test_platformName(new_session, add_browser_capabilities): no mixed camelCase
Attachment #8995952 -
Flags: review?(hskupin) → review+
Comment 34•6 years ago
|
||
Comment on attachment 8995954 [details] [diff] [review] Parametrize capabilities tests. Review of attachment 8995954 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/new_session/response.py @@ +19,5 @@ > + ("acceptInsecureCerts", bool), > + ("pageLoadStrategy", basestring), > + ("proxy", dict), > + ("setWindowRect", bool), > + ("timeouts", dict), It would still be good if you could add `unhandledPromptBehavior` here too as pointed out in my last review. @@ +21,5 @@ > + ("proxy", dict), > + ("setWindowRect", bool), > + ("timeouts", dict), > +]) > +def test_capability_type(session, add_browser_capabilities, capability, type): `add_browser_capabilities` is unused @@ +34,5 @@ > + ("proxy", {}), > + ("setWindowRect", True), > + ("timeouts", {"implicit": 0, "pageLoad": 300000, "script": 30000}), > +]) > +def test_capability_default_value(session, add_browser_capabilities, capability, default_value): `add_browser_capabilities` is unused
Attachment #8995954 -
Flags: review?(hskupin) → review+
Comment 35•6 years ago
|
||
Comment on attachment 8995953 [details] [diff] [review] Move timeouts test to separate file. Review of attachment 8995953 [details] [diff] [review]: ----------------------------------------------------------------- I assume this patch is ready to be reviewed and you only missed to set the reviewer. ::: testing/web-platform/tests/webdriver/tests/new_session/timeouts.py @@ +3,5 @@ > +from tests.support.asserts import assert_success > + > + > +def test_default_values(new_session, add_browser_capabilities): > + response, _ = new_session({"capabilities": {"alwaysMatch": add_browser_capabilities({})}}) Same as for platform. Use the `session` capability to not force a restart. @@ +17,5 @@ > + {"implicit": 444, "pageLoad": 300000,"script": 30000}, > + {"implicit": 0, "pageLoad": 444,"script": 30000}, > + {"implicit": 0, "pageLoad": 300000,"script": 444}, > +]) > +def test_timeouts(new_session, add_browser_capabilities, timeouts): Same here.
Attachment #8995953 -
Flags: review-
Assignee | ||
Comment 36•6 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #33) > Comment on attachment 8995952 [details] [diff] [review] > Move platformName test to separate file. > > Review of attachment 8995952 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: testing/web-platform/tests/webdriver/tests/new_session/platform_name.py > @@ +1,4 @@ > > +import pytest > > + > > +from tests.support.asserts import assert_success > > +from tests.support import platform_name > > why not flipped? They _are_ sorted according to lexicographical order, e.g. sort(1). In any case, this is not crucial to the function of the patch. > @@ +3,5 @@ > > +from tests.support.asserts import assert_success > > +from tests.support import platform_name > > + > > + > > +@pytest.mark.skip_if(platform_name is None, reason="Unsupported platform %s" % platform_name) > > Lets prefer to use `format` all over the place. Why? AIUI they are functionally equivalent. > @@ +4,5 @@ > > +from tests.support import platform_name > > + > > + > > +@pytest.mark.skip_if(platform_name is None, reason="Unsupported platform %s" % platform_name) > > +def test_platformName(new_session, add_browser_capabilities): > > no mixed camelCase Here the test name is referring to a specific symbol so I don’t think we should rewrite it, but maybe we can change the test title to something more meaningful considering the file’s name is already platform_name.py. I’ve put test_corresponds_to_local_system there now which I think will read better in the results table as it talks about what the test is specifically testing.
Assignee | ||
Comment 37•6 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #34) > ::: testing/web-platform/tests/webdriver/tests/new_session/response.py > @@ +19,5 @@ > > + ("acceptInsecureCerts", bool), > > + ("pageLoadStrategy", basestring), > > + ("proxy", dict), > > + ("setWindowRect", bool), > > + ("timeouts", dict), > > It would still be good if you could add `unhandledPromptBehavior` here too > as pointed out in my last review. This was not already in the original test and really should be addressed separately, but I’ve added it now.
Assignee | ||
Comment 38•6 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #35) > Comment on attachment 8995953 [details] [diff] [review] > Move timeouts test to separate file. > > Review of attachment 8995953 [details] [diff] [review]: > ----------------------------------------------------------------- > > I assume this patch is ready to be reviewed and you only missed to set the > reviewer. Both you and AutomatedTester previously gave this patch r+…
Assignee | ||
Comment 39•6 years ago
|
||
Change the switch-statement to only do assertions, and replace the v variable with any parsed values before writing it to the matched set of capabilities in one location.
Attachment #8996318 -
Flags: review?(hskupin)
Assignee | ||
Updated•6 years ago
|
Attachment #8995949 -
Attachment is obsolete: true
Assignee | ||
Comment 40•6 years ago
|
||
The setWindowRect capability is mandated by the WebDriver standard and is an indication whether the driver supports manipulating the window dimensions and position. This will always be true for Firefox and always false for Fennec.
Attachment #8996319 -
Flags: review?(hskupin)
Assignee | ||
Updated•6 years ago
|
Attachment #8995950 -
Attachment is obsolete: true
Assignee | ||
Comment 41•6 years ago
|
||
It will never be possible to configure setWindowRect, and trying to do so will cause geckodriver to return with an error.
Attachment #8996320 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8995951 -
Attachment is obsolete: true
Assignee | ||
Comment 42•6 years ago
|
||
Specific tests for the platformName capability do not belong in the test for the response body structure.
Attachment #8995952 -
Attachment is obsolete: true
Attachment #8996321 -
Flags: review?(hskupin)
Assignee | ||
Comment 43•6 years ago
|
||
For similar reasons as for platformName, tests for configuring the timeouts object do not belong in the same parent test as those for response body structure.
Attachment #8996322 -
Flags: review?(hskupin)
Attachment #8996322 -
Flags: review?(dburns)
Assignee | ||
Updated•6 years ago
|
Attachment #8994013 -
Attachment is obsolete: true
Attachment #8995953 -
Attachment is obsolete: true
Assignee | ||
Comment 44•6 years ago
|
||
To be able to ignore tests for individual capabilities we need to parametrize these tests. geckodriver now supports setWindowRect, but fails the proxy capability test because it is for some reason not propagated back.
Attachment #8996323 -
Flags: review?(hskupin)
Assignee | ||
Updated•6 years ago
|
Attachment #8995954 -
Attachment is obsolete: true
Assignee | ||
Comment 45•6 years ago
|
||
Attachment #8995955 -
Attachment is obsolete: true
Attachment #8996324 -
Flags: review+
Assignee | ||
Comment 46•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2836194ccae76068547cdcd0ae16a6a2d1de94d4
Comment 47•6 years ago
|
||
(In reply to Andreas Tolfsen 「:ato」 from comment #36) > > > +from tests.support.asserts import assert_success > > > +from tests.support import platform_name > > > > why not flipped? > > They _are_ sorted according to lexicographical order, e.g. sort(1). sort clearly doesn't sort it this way for me on MacOS. > > Lets prefer to use `format` all over the place. > > Why? AIUI they are functionally equivalent. While the old style formatting is not deprecated yet, it is recommended for Python 3 to use the new style. > > > +@pytest.mark.skip_if(platform_name is None, reason="Unsupported platform %s" % platform_name) > > > +def test_platformName(new_session, add_browser_capabilities): > > > > no mixed camelCase > > Here the test name is referring to a specific symbol so I don’t > think we should rewrite it, but maybe we can change the test title > to something more meaningful considering the file’s name is already > platform_name.py. I’ve put test_corresponds_to_local_system there > now which I think will read better in the results table as it talks > about what the test is specifically testing. Didn't this cause a linter failure before?
Updated•6 years ago
|
Attachment #8996318 -
Flags: review?(hskupin) → review+
Updated•6 years ago
|
Attachment #8996319 -
Flags: review?(hskupin) → review+
Updated•6 years ago
|
Attachment #8996321 -
Flags: review?(hskupin) → review+
Updated•6 years ago
|
Attachment #8996322 -
Flags: review?(hskupin) → review+
Updated•6 years ago
|
Attachment #8996323 -
Flags: review?(hskupin) → review+
Updated•6 years ago
|
Attachment #8996320 -
Flags: review?(dburns) → review+
Updated•6 years ago
|
Attachment #8996322 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 48•6 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #47) > (In reply to Andreas Tolfsen 「:ato」 from comment #36) > > > > > +from tests.support.asserts import assert_success import > > > > +from tests.supportplatform_name > > > > > > why not flipped? > > > > They _are_ sorted according to lexicographical order, e.g. > > sort(1). > > sort clearly doesn't sort it this way for me on MacOS. I noticed that too. Using "LC_ALL=en_US.utf8 sort" appears to fix the issue on my Linux workstation. > > > Lets prefer to use `format` all over the place. > > > > Why? AIUI they are functionally equivalent. > > While the old style formatting is not deprecated yet, it is > recommended for Python 3 to use the new style. OK, have updated them all.
Assignee | ||
Comment 49•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=553048b2502d8ca3a53bd267c644d01dcf873e56
Comment 50•6 years ago
|
||
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/39954f25c48e Simplify Marionette capability parsing. r=whimboo https://hg.mozilla.org/integration/mozilla-inbound/rev/203d1449aa59 Add setWindowRect capability to Marionette. r=whimboo https://hg.mozilla.org/integration/mozilla-inbound/rev/e99e43a1e400 Add setWindowRect capability to geckodriver. r=automatedtester https://hg.mozilla.org/integration/mozilla-inbound/rev/a3d141d56e53 Move platformName test to separate file. r=whimboo https://hg.mozilla.org/integration/mozilla-inbound/rev/0ad9872c27e0 Move timeouts test to separate file. r=automatedtester,whimboo https://hg.mozilla.org/integration/mozilla-inbound/rev/b5d9ec944fe0 Parametrize capabilities tests. r=whimboo https://hg.mozilla.org/integration/mozilla-inbound/rev/b04c9a64cd61 Regenerate WPT manifest. r=me
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12260 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/12260 * continuous-integration/travis-ci/pr (https://travis-ci.org/web-platform-tests/wpt/builds/410923358?utm_source=github_status&utm_medium=notification)
Comment 53•6 years ago
|
||
(In reply to Andreas Tolfsen 「:ato」 from comment #48) > OK, have updated them all. Thanks!
Comment 54•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/39954f25c48e https://hg.mozilla.org/mozilla-central/rev/203d1449aa59 https://hg.mozilla.org/mozilla-central/rev/e99e43a1e400 https://hg.mozilla.org/mozilla-central/rev/a3d141d56e53 https://hg.mozilla.org/mozilla-central/rev/0ad9872c27e0 https://hg.mozilla.org/mozilla-central/rev/b5d9ec944fe0 https://hg.mozilla.org/mozilla-central/rev/b04c9a64cd61
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Upstream PR merged
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•