940554 - newSession should return session ID, available capabilities, and status

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

Attachment: 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`.

Comment 1

[David Burns :automatedtester]

>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>

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: 0001-Bug-940554-Fix-Marionette-s-newSession-to-return-cap.patch

try: https://tbpl.mozilla.org/?tree=Try&rev=14c0dc0ea320

Comment 3

[Malini Das [:mdas] - Away, not checking bugmail]

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!

Comment 4

[Andreas Tolfsen ❲:ato❳]

Thanks! \o/

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e5dfc4abbb91

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if 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.

Comment 7

[Andreas Tolfsen ❲:ato❳]

Attachment: 0001-Bug-940554-Fix-Marionette-s-newSession-to-return-cap.patch

mobile try: https://tbpl.mozilla.org/?tree=Try&rev=86f2106aae0e

Comment 8

[Andreas Tolfsen ❲:ato❳]

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.

Comment 9

[Andreas Tolfsen ❲:ato❳]

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.

Comment 10

[Andreas Tolfsen ❲:ato❳]

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

Comment 11

[Andreas Tolfsen ❲:ato❳]

Intermittent (?) Mnw failures, triggered new try run for mobile: https://tbpl.mozilla.org/?tree=Try&rev=3ad45b97b781

Comment 12

[Malini Das [:mdas] - Away, not checking bugmail]

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>

Comment 13

[Andreas Tolfsen ❲:ato❳]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Bitrotted, please rebase.

Comment 15

[Andreas Tolfsen ❲:ato❳]

Attachment: 0001-Bug-940554-Fix-Marionette-s-newSession-to-return-cap.patch

Rebased patch, carrying on r+ from mdas.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d1c157bbb46

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Comment 18

[Andreas Tolfsen ❲:ato❳]

Attachment: 0001-Bug-940554-Fix-Marionette-s-newSession-to-return-cap.patch

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.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ec1142c5540

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3ec1142c5540