newSession should return session ID, available capabilities, and status

RESOLVED FIXED in mozilla30

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: ato, Assigned: ato)

Tracking

(Blocks 1 bug)

Trunk
mozilla30
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Assignee

Description

6 years ago
Posted file test_new_session.py
According to the specification the newSession command must return a
response object containing at least:

  * "sessionId": the session ID associated with this session
  * "value": a capabilities instance
  * "status": the success/error code

Currently it only returns what seems to be the status:

    1384881334627   Marionette      INFO    sendToClient: {"from":"0","value":"19"}, {8cfa1d06-d24f-48e9-8be4-891b934f7374}, {8cfa1d06-d24f-48e9-8be4-891b934f7374}

We'd expect at least the following:

    {"value": {"browserName": …, "browserVersion": …, "platformName": …, "platformVersion": …},
     "sessionId": …,
     "status": …}

Unless this information is sent, it could be difficult for strongly
typed client bindings to know if the driver can be augmented or not
(for example to take screenshots or change screen orientation).

Please see the attached TC, which can be executed doing `python
test_new_session.py`.
Assignee

Updated

6 years ago
Summary: newSession returns incorrect response → newSession should return session ID, available capabilities, and status
>Unless this information is sent, it could be difficult for strongly
>typed client bindings to know if the driver can be augmented or not
>(for example to take screenshots or change screen orientation).

<troll>
This shows the limitation in the currently Java bindings and why role based APIs don't always work when you need to write augmentors to do basic things
</troll>
Assignee

Updated

5 years ago
Assignee: nobody → ato
Status: NEW → ASSIGNED
Comment on attachment 8366133 [details] [diff] [review]
0001-Bug-940554-Fix-Marionette-s-newSession-to-return-cap.patch

Review of attachment 8366133 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm thanks!
Attachment #8366133 - Flags: review?(mdas) → review+
Assignee

Comment 4

5 years ago
Thanks! \o/
Keywords: checkin-needed
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3b88bcc4c3d4 for causing b2g test failures across the board like these: 
https://tbpl.mozilla.org/php/getParsedLog.php?id=33713656&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=33713598&tree=Mozilla-Inbound

07:13:28     INFO -  REFTEST INFO | runreftest.py | Running tests: start.
07:14:13     INFO -  Traceback (most recent call last):
07:14:13     INFO -    File "runreftestb2g.py", line 573, in run_remote_reftests
07:14:13     INFO -      retVal = reftest.runTests(manifest, options, cmdlineArgs)
07:14:13     INFO -    File "/home/cltbld/talos-slave/test/build/tests/reftest/runreftest.py", line 225, in runTests
07:14:13     INFO -      return self.runSerialTests(testPath, options, cmdlineArgs)
07:14:13     INFO -    File "/home/cltbld/talos-slave/test/build/tests/reftest/runreftest.py", line 321, in runSerialTests
07:14:13     INFO -      timeout=options.timeout + 30.0)
07:14:13     INFO -    File "/home/cltbld/talos-slave/test/build/tests/reftest/automation.py", line 844, in runApp
07:14:13     INFO -      stderr = subprocess.STDOUT)
07:14:13     INFO -    File "/home/cltbld/talos-slave/test/build/tests/reftest/b2gautomation.py", line 259, in Process
07:14:13     INFO -      raise Exception("bad session value %s returned by start_session" % session)
07:14:13     INFO -  Exception: bad session value {u'rotatable': True, u'takesElementScreenshot': True, u'takesScreenshot': True, u'appBuildId': u'20140128151741', u'cssSelectorsEnabled': True, u'javascriptEnabled': True, u'XULappId': u'{3c2e2abc-06d4-11e1-ac3b-374f68613e61}', u'secureSsl': False, u'device': u'qemu', u'browserName': u'B2G', u'version': u'29.0a1', u'nativeEvents': False, u'platformVersion': u'29.0a1', u'platformName': u'Android', u'handlesAlerts': False} returned by start_session
07:14:14     INFO -  Automation Error: Exception caught while running tests
07:14:14    ERROR - Return code: 1





Next try run should probably actually run tests.
Flags: in-testsuite+
Assignee

Comment 8

5 years ago
New patch reintroduces an additional "b2g" property in the dict of
capabilities for backwards compatibility with eideticker (bug 965297)
and mochitest (bug 965304).

I intended to remove it as part of bug 965308.
Assignee

Updated

5 years ago
Blocks: 965291
Assignee

Comment 9

5 years ago
Comment on attachment 8367367 [details] [diff] [review]
0001-Bug-940554-Fix-Marionette-s-newSession-to-return-cap.patch

The revised patch adds a "b2g" property to the capabilities which should satisfy eideticker's and mochitest's "'b2g' in marionette.session" check.
Attachment #8367367 - Flags: review?(mdas)
Attachment #8367367 - Flags: review?(mdas) → review+
Assignee

Comment 10

5 years ago
full try run with mobile this time: https://tbpl.mozilla.org/?tree=Try&rev=56b695183a35
Assignee

Comment 11

5 years ago
Intermittent (?) Mnw failures, triggered new try run for mobile: https://tbpl.mozilla.org/?tree=Try&rev=3ad45b97b781
Can you rerun Gu as well? there are some intermittents there, but also failures I've never seen like
08:33:03    ERROR -  TEST-UNEXPECTED-FAIL | test_killall.py test_killall.TestKillAll.test_kill_all | TimeoutException: <unprintable TimeoutException object>
Assignee

Comment 13

5 years ago
Triggered another try run with a rebased version of the patch to verify that it doesn't pick up marionette_client 0.7.2 for the Gu tests: https://tbpl.mozilla.org/?tree=Try&rev=776c3d79b457
Assignee

Updated

5 years ago
Keywords: checkin-needed
Bitrotted, please rebase.
Keywords: checkin-needed
Assignee

Comment 15

5 years ago
Rebased patch, carrying on r+ from mdas.
Attachment #8367367 - Attachment is obsolete: true
Attachment #8371411 - Flags: review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
Backed out for the changes it made to the script timeout message per our discussion in #ateam.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7dbb42dd1cf4
Assignee

Comment 18

5 years ago
Updated patch to not pass the wrong argument type to the TimeoutException's cause argument.  This should take care of the unprintable exception problem, which was really caused by a TypeError inside the exception's string representation.
Attachment #8371411 - Attachment is obsolete: true
Attachment #8372280 - Flags: review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3ec1142c5540
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.