1196987 - Marionette should return valid UUID

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[David Burns :automatedtester]

STR

    POST /session with

{"desiredCapabilities":{"browserName":"firefox","name":"intern.js","idle-timeout":60}}

Actual

wires response

{
  "sessionId": "{e55fd540-babf-3548-86e8-2e20b8fb3bb1}",
  "value": {
    "XULappId": "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}",
    "acceptSslCerts": false,
    "appBuildId": "20150819030206",
    "browserName": "Firefox",
    "browserVersion": "43.0a1",
    "device": "desktop",
    "platform": "DARWIN",
    "platformName": "DARWIN",
    "platformVersion": "43.0a1",
    "proxy": {},
    "raisesAccessibilityExceptions": false,
    "rotatable": false,
    "takesElementScreenshot": true,
    "takesScreenshot": true,
    "version": "43.0a1"
  }
}

Expected

Expected to have sessionId to be a valid uuid.v4 ( https://en.wikipedia.org/wiki/Universally_unique_identifier ) without { }

Selenium response

{
  "sessionId": "b510d3dd-5939-4b0e-a523-1a1a90498b0d",
  "status": 0,
  "state": null,
  "value": {
    "platform": "MAC",
    "javascriptEnabled": true,
    "acceptSslCerts": true,
    "browserName": "firefox",
    "rotatable": false,
    "locationContextEnabled": true,
    "webdriver.remote.sessionid": "b510d3dd-5939-4b0e-a523-1a1a90498b0d",
    "version": "40.0",
    "databaseEnabled": true,
    "cssSelectorsEnabled": true,
    "handlesAlerts": true,
    "webStorageEnabled": true,
    "nativeEvents": false,
    "applicationCacheEnabled": true,
    "takesScreenshot": true
  },
  "class": "org.openqa.selenium.remote.Response",
  "hCode": 1424204968
}

Comment 1

[David Burns :automatedtester]

Attachment: MozReview Request: Bug 1196987: Update Marionette Session IDs to be valid UUID. r?ato

Bug 1196987: Update UUID returned to be a valid UUID 4; r?jgriffin

Currently the UUID returns the `{}` which is not valid and breaks
URI Templates used in WebDriver languages

Comment 2

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8652010 [details]
MozReview Request: Bug 1196987: Update Marionette Session IDs to be valid UUID. r?ato

https://reviewboard.mozilla.org/r/17047/#review15179

Ship It!

Comment 3

[David Burns :automatedtester]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/f56c58238801ee4886bb4da3d64b33e729bead0f
changeset:  f56c58238801ee4886bb4da3d64b33e729bead0f
user:       David Burns <[e-mail]>
date:       Mon Aug 24 23:15:46 2015 +0100
description:
Bug 1196987: Update UUID returned to be a valid UUID 4; r=jgriffin

Currently the UUID returns the `{}` which is not valid and breaks
URI Templates used in WebDriver languages

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f56c58238801

Comment 5

[David Burns :automatedtester]

Please sir, can you uplift to aurora a=test-only

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/32e63d193254

Comment 7

[David Burns :automatedtester]

Comment on attachment 8652010 [details]
MozReview Request: Bug 1196987: Update Marionette Session IDs to be valid UUID. r?ato

Bug 1196987: Update Marionette element IDs to be valid UUID. r?ato

This allows WebDriver compatibility to create valid URI.

Comment 8

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8652010 [details]
MozReview Request: Bug 1196987: Update Marionette Session IDs to be valid UUID. r?ato

https://reviewboard.mozilla.org/r/17047/#review15345

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:167
(Diff revision 2)
> +        self.assertNotIn("{", el.id, 'element id has {{ in it. Id has {}'.format(el.id))

The regular expression

    /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i

would probably be a better test since it checks that the UUID is compatible with RFC 4122 version 1 through 5 according to [this answer](http://stackoverflow.com/questions/7905929/how-to-test-valid-uuid-guid) on Stackoverflow.

::: testing/marionette/elements.js:236
(Diff revision 2)
> -    let id = uuidGen.generateUUID().toString();
> +    let uuid = uuidGen.generateUUID().toString();
> +    let id = uuid.substring(1, uuid.length - 1);

It would be nice if this was made into a library function we shared across files, because I will need to use this when the `ListenerProxy` starts generating its own UUID internally instead of passing along command ID.

I didn’t open an issue on this, so feel free to fix or not fix.  I’ll get around to it when I do my patch to the proxy anyway.