Return no such session id error when there is no session

RESOLVED FIXED in Firefox 52

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ato, Assigned: ato)

Tracking

(Blocks: 1 bug)

Version 3
mozilla54
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 fixed, firefox53 fixed, firefox54 fixed)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Marionette should return a ‘no such session id’ error when the Delete Session command is called and there there is no current session.
(Assignee)

Updated

2 years ago
Assignee: nobody → ato
Blocks: 721859
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8833339 - Attachment is obsolete: true
Attachment #8833339 - Flags: review?(mjzffr)
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED

Comment 14

2 years ago
mozreview-review
Comment on attachment 8832916 [details]
Bug 1336124 - Add assert.session for checking if session is active;

https://reviewboard.mozilla.org/r/109166/#review110786

::: testing/marionette/assert.js:39
(Diff revision 2)
> + *
> + * @throws {InvalidSessionIdError}
> + *     If |driver| does not have a session ID.
> + */
> +assert.session = function (driver, msg = "") {
> +  assert.that(sessionID => !!sessionID,

Why not just `sessionID => sessionID`?

::: testing/marionette/test_assert.js:16
(Diff revision 2)
>  
> +add_test(function test_session() {
> +  assert.session({sessionId: "foo"});
> +  Assert.throws(() => assert.session({sessionId: null}), InvalidSessionIdError);
> +  Assert.throws(() => assert.session({sessionId: undefined}), InvalidSessionIdError);
> +

Doesn't hurt to check `""`, too.
Attachment #8832916 - Flags: review?(mjzffr) → review+

Comment 15

2 years ago
mozreview-review
Comment on attachment 8832917 [details]
Bug 1336124 - Remove unused B2G code in session teardown;

https://reviewboard.mozilla.org/r/109168/#review110788
Attachment #8832917 - Flags: review?(mjzffr) → review+

Comment 16

2 years ago
mozreview-review
Comment on attachment 8832918 [details]
Bug 1336124 - Rename sessionTeardown to deleteSession;

https://reviewboard.mozilla.org/r/109170/#review110792
Attachment #8832918 - Flags: review?(mjzffr) → review+

Comment 17

2 years ago
mozreview-review
Comment on attachment 8832919 [details]
Bug 1336124 - Return error when there is no session;

https://reviewboard.mozilla.org/r/109172/#review110798
Attachment #8832919 - Flags: review?(mjzffr) → review+

Comment 18

2 years ago
mozreview-review
Comment on attachment 8833340 [details]
Bug 1336124 - Only delete session if one exists when testing capabilities;

https://reviewboard.mozilla.org/r/109598/#review110800
Attachment #8833340 - Flags: review?(mjzffr) → review+
(Assignee)

Comment 19

2 years ago
mozreview-review-reply
Comment on attachment 8832916 [details]
Bug 1336124 - Add assert.session for checking if session is active;

https://reviewboard.mozilla.org/r/109166/#review110786

> Why not just `sessionID => sessionID`?

Good point.

> Doesn't hurt to check `""`, too.

Indeed.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 25

2 years ago
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/589a01fb4790
Add assert.session for checking if session is active; r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/c93a410adde0
Remove unused B2G code in session teardown; r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/ca0345ae33a4
Rename sessionTeardown to deleteSession; r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/6d17a85df7e3
Return error when there is no session; r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/c4441c1492fd
Only delete session if one exists when testing capabilities; r=maja_zf
(Assignee)

Comment 27

2 years ago
Sheriffs: Please uplift to Aurora and Beta as testonly.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.