Closed Bug 1326174 Opened 8 years ago Closed 8 years ago

For unsupported commands in chrome context throw UnsupportedOperationError

Categories

(Remote Protocol :: Marionette, defect)

Product:
Remote Protocol
Component:
Marionette
Version:
Version 3
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox52 fixed, firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [spec][17/01])

Attachments

(2 files)

Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError
59 bytes, text/x-review-board-request
ato
: review+
Details
Bug 1326174 - Handle cookies with content scope in test_utils.py
59 bytes, text/x-review-board-request
impossibus
: review+
Details 
Some of the webdriver commands do not throw a UnsupportedOperationError exception in case the command gets issued in chrome context.
Attachment #8822419 - Flags: review?(ato)
Comment on attachment 8822419 [details]
Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError

https://reviewboard.mozilla.org/r/101338/#review101902

::: testing/marionette/assert.js:81
(Diff revision 2)
> + *
> + * @throws {UnsupportedOperationError}
> + *     If |context| is not content.
> + */
> +assert.content = function (context, msg = "") {
> +  msg = msg || error.pprint`Expected ${context} to be content`;

I wonder if this should simply say “Command not supported in chrome context” so we don’t have to override the message every time.  I don’t think there is any value in saying which _specific_ command was not available.

::: testing/marionette/driver.js:2530
(Diff revision 2)
>    ];
>  
>    let or = String(cmd.parameters.orientation);
>    assert.string(or);
>    let mozOr = or.toLowerCase();
> -  if (!ors.include(mozOr)) {
> +  if (!ors.includes(mozOr)) {

Well spotted.
Attachment #8822419 - Flags: review?(ato) → review+
Comment on attachment 8822419 [details]
Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError

https://reviewboard.mozilla.org/r/101338/#review101902

> I wonder if this should simply say “Command not supported in chrome context” so we don’t have to override the message every time.  I don’t think there is any value in saying which _specific_ command was not available.

That makes sense. I will correct it.

> Well spotted.

Well, it was hiding for Fennec until I landed my skip decorator fix earlier today. So I have seen it here:

https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=mn&bugfiler&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=65287721

I actually may want to split this out to a separate commit.
Comment on attachment 8822419 [details]
Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError

https://reviewboard.mozilla.org/r/101338/#review101902

> That makes sense. I will correct it.

I assume you are fine if I also correct this for assert.[firefox,fennec,mobile].
(In reply to Henrik Skupin (:whimboo) from comment #5)
> I assume you are fine if I also correct this for
> assert.[firefox,fennec,mobile].

Can you please have a look again? Thanks.
Flags: needinfo?(ato)
r++
Flags: needinfo?(ato)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57a62a57c996
For unsupported commands in chrome context throw UnsupportedOperationError r=ato
So the utils.py Puppeteer unit test indeed failed because it was using the delete_all_cookies() method in chrome scope. I fixed it locally and will push another try build.
Flags: needinfo?(hskupin)
Attachment #8822532 - Flags: review?(mjzffr)
Comment on attachment 8822532 [details]
Bug 1326174 - Handle cookies with content scope in test_utils.py

https://reviewboard.mozilla.org/r/101408/#review102038
Attachment #8822532 - Flags: review?(mjzffr) → review+
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cd1da60be54
Handle cookies with content scope in test_utils.py r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/3e78b4df8dd7
For unsupported commands in chrome context throw UnsupportedOperationError r=ato
https://hg.mozilla.org/mozilla-central/rev/2cd1da60be54
https://hg.mozilla.org/mozilla-central/rev/3e78b4df8dd7
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Test-only patch which we need to uplift to aurora. Thanks.
status-firefox52: --- → affected
Whiteboard: [checkin-needed-aurora]
the 2nd patch need rebasing for aurora
Flags: needinfo?(hskupin)
(In reply to Carsten Book [:Tomcat] from comment #18)
> the 2nd patch need rebasing for aurora

I pulled latest aurora code and tried to graft both commits which worked fine. Can you please check again what was wrong with your graft command?
Flags: needinfo?(hskupin)
Whiteboard: [spec][17/01]
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: