Andreas Tolfsen ❲:ato❳ Assignee
	Description • 7 years ago

Since https://bugzilla.mozilla.org/show_bug.cgi?id=1387380 capabilities received from geckodriver are not recognised as can be seen from https://gist.github.com/barancev/bf99c3d49045025dbb02763a055dfe59#file-gistfile1-txt-L216-L218:

> 1502207295956	geckodriver::marionette	TRACE	-> 215:[0,1,"newSession",{"acceptInsecureCerts":true,"browserName":"firefox","capabilities":{"desiredCapabilities":{"acceptInsecureCerts":true,"browserName":"firefox","pageLoadStrategy":"none"}},"pageLoadStrategy":"none"}]
> 1502207295958	Marionette	TRACE	0 -> [0,1,"newSession",{"acceptInsecureCerts":true,"browserName":"firefox","capabilities":{"desiredCapabilities":{"acceptInsecureCerts":true,"browserName":"firefox","pageLoadStrategy":"none"}},"pageLoadStrategy":"none"}]
> 1502207295989	Marionette	DEBUG	Register listener.js for window 4294967297
> 1502207296010	Marionette	TRACE	0 <- [1,1,null,{"sessionId":"dd9b56a9-5e0f-4e74-a752-330fe0719d3e","capabilities":{"browserName":"firefox","browserVersion":"57.0a1","platformName":"windows_nt","platformVersion":"6.1","pageLoadStrategy":"normal","acceptInsecureCerts":false,"timeouts":{"implicit":0,"pageLoad":300000,"script":30000},"rotatable":false,"specificationLevel":0,"moz:processID":2756,"moz:profile":"C:\\Users\\alexei\\AppData\\Local\\Temp\\rust_mozprofile.j8M8BAyUlCNn","moz:accessibilityChecks":false,"moz:headless":false}}]
> 1502207296011 geckodriver::marionette TRACE <- [1,1,null,{"sessionId":"dd9b56a9-5e0f-4e74-a752-330fe0719d3e","capabilities":{"browserName":"firefox","browserVersion":"57.0a1","platformName":"windows_nt","platformVersion":"6.1","pageLoadStrategy":"normal","acceptInsecureCerts":false,"timeouts":{"implicit":0,"pageLoad":300000,"script":30000},"rotatable":false,"specificationLevel":0,"moz:processID":2756,"moz:profile":"C:\\Users\\alexei\\AppData\\Local\\Temp\\rust_mozprofile.j8M8BAyUlCNn","moz:accessibilityChecks":false,"moz:headless":false}}]

geckodriver sends the capabilities at the top-level of the command parameters element as well as in the backwards compat field:

> [0,1,"newSession",{"pageLoadStrategy": "none", "capabilities": {"desiredCapabilities": {"pageLoadStrategy": "none"}}, …}]

But since bug 1387380, Marionette only reads the "capabilities" field off this JSON Object:

>     this.capabilities = session.Capabilities.fromJSON(
>         cmd.parameters.capabilities);

This is a bug and it should read cmd.parameters instead.

Unfortunately, Marionette unit tests expect {sessionId: <old session ID>, capabilities: <dict>} to be the data format.  I think we should remove the ability to override the session ID.  As far as I can tell, this functionality is only used for in-app restart tests, but it’s arguably not the same Marionette session when Firefox has restarted.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 2 • 7 years ago

(In reply to Andreas Tolfsen ‹:ato› from comment #0)
> ability to override the session ID.  As far as I can tell, this
> functionality is only used for in-app restart tests, but it’s arguably not
> the same Marionette session when Firefox has restarted.

I'm happy to get this removed.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 3 • 7 years ago
mozreview-review

Comment on attachment 8894975 [details]
Bug 1388424 - Read capabilities off top-level object.

https://reviewboard.mozilla.org/r/166086/#review171294

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 6 • 7 years ago

Slight update because I was copying a reference, causing circular references in the Python dictionaries.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 7 • 7 years ago

I assume this is done and we can land. It blocks my work, so I pushed it to autoland.

	Pulsebot
	Comment 8 • 7 years ago

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cf4290c2eef
Read capabilities off top-level object. r=whimboo

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 9 • 7 years ago

Backed out for failing marionette's test_quit_restart.py TestQuitRestart.test_force_clean_restart:

https://hg.mozilla.org/integration/autoland/rev/d3cb33a38211dfb131958fec2a52cb27bf4abc49

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2cf4290c2eeff7dd6f3c6c21e8393cb3670ca9b3&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122516677&repo=autoland

[task 2017-08-11T09:45:33.397142Z] 09:45:33     INFO -  1502444733377	Marionette	TRACE	2 -> [0,6,"setContext",{"value":"content"}]
[task 2017-08-11T09:45:33.397426Z] 09:45:33     INFO -  1502444733378	Marionette	TRACE	2 <- [1,6,null,{}]
[task 2017-08-11T09:45:33.397817Z] 09:45:33     INFO -  1502444733379	Marionette	TRACE	2 -> [0,7,"getContext",{}]
[task 2017-08-11T09:45:33.398346Z] 09:45:33     INFO -  1502444733380	Marionette	TRACE	2 <- [1,7,null,{"value":"content"}]
[task 2017-08-11T09:45:33.404980Z] 09:45:33     INFO -  1502444733381	Marionette	TRACE	2 -> [0,8,"setContext",{"value":"content"}]
[task 2017-08-11T09:45:33.405211Z] 09:45:33     INFO -  1502444733382	Marionette	TRACE	2 <- [1,8,null,{}]
[task 2017-08-11T09:45:33.405580Z] 09:45:33     INFO -  1502444733384	Marionette	TRACE	2 -> [0,9,"getPageSource",{}]
[task 2017-08-11T09:45:33.406367Z] 09:45:33     INFO -  1502444733396	Marionette	TRACE	2 <- [1,9,null,{"value":"<html><head></head><body></body></html>"}]
[task 2017-08-11T09:45:33.407403Z] 09:45:33     INFO -  1502444733400	Marionette	TRACE	2 -> [0,10,"setContext",{"value":"content"}]
[task 2017-08-11T09:45:33.419312Z] 09:45:33     INFO - TEST-UNEXPECTED-FAIL | test_quit_restart.py TestQuitRestart.test_force_clean_restart | AssertionError: u'dcdb0ce1-7513-46bc-bb22-88fd6ac65220' != u'46f9a717-7220-4b38-9b3c-98515dc561f5'
[task 2017-08-11T09:45:33.419554Z] 09:45:33     INFO - - dcdb0ce1-7513-46bc-bb22-88fd6ac65220
[task 2017-08-11T09:45:33.419644Z] 09:45:33     INFO - + 46f9a717-7220-4b38-9b3c-98515dc561f5
[task 2017-08-11T09:45:33.419723Z] 09:45:33     INFO - Traceback (most recent call last):
[task 2017-08-11T09:45:33.420061Z] 09:45:33     INFO -   File "/home/worker/workspace/build/venv/local/lib/python2.7/site-packages/marionette_harness/marionette_test/testcases.py", line 156, in run
[task 2017-08-11T09:45:33.420216Z] 09:45:33     INFO -     testMethod()
[task 2017-08-11T09:45:33.420365Z] 09:45:33     INFO -   File "/home/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py", line 105, in test_force_clean_restart
[task 2017-08-11T09:45:33.421258Z] 09:45:33     INFO -     self.assertEqual(self.marionette.session_id, self.session_id)

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 10 • 7 years ago

The problem here is that the patch missed to update both of the forced restart tests which are failing now because the session id is reset in between:

test_quit_restart.py test_quit_restart.TestQuitRestart.test_force_clean_restart
test_quit_restart.py test_quit_restart.TestQuitRestart.test_force_restart

I will fix it locally and upload a patch which we can then land via inbound.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 12 • 7 years ago

I triggered a try push of the latest version of the patch together with my changes on bug 1387092:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=08bd74be88a2

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 13 • 7 years ago

The try build is green. Can someone please land attachment 8896206 [details] [diff] [review] on inbound? Thanks.

	Pulsebot
	Comment 14 • 7 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/923bb6366e72
Read capabilities off top-level object. r=whimboo

	Pulsebot
	Comment 16 • 7 years ago

Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fce0b08bf07c
Fix flake8 issue a=me

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 17 • 7 years ago

Thanks for picking this up whilst I’ve been away, Henrik.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 18 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/923bb6366e72
https://hg.mozilla.org/mozilla-central/rev/fce0b08bf07c