964738 - Upgrade atoms to latest Selenium export

Status: RESOLVED INVALID

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

We should probably update the Selenium atoms in testing/marionette/atoms/atoms.js to the ones from the latest Selenium release.

Comment 1

[Andreas Tolfsen ❲:ato❳]

A while back I landed a patch adding a build target for Marionette atoms to Selenium.  Unfortunately I'm unsure if I've picked the right atom targets:

    https://code.google.com/p/selenium/source/detail?r=2f47941a58fb7bec448f6492a3d2b478c2f0a8c4

An initial test I did with the newly generated atoms.js file from `./go marionette:atoms` against the Marionette unit test suite showed no regressions.  However, I'm not familiar with the Gecko .jar packaging system.  If I replace testing/marionette/atoms/atoms.js and run `mach build`, will this trigger the right paths in the dependency graph to rebuild Firefox properly?

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: atoms.js

Attaching a fresh atoms.js file generated from 948fbeddfd6739b40a7348a8e2dd5e4e58a4d70c in Selenium.

Comment 3

[David Burns :automatedtester]

What changes in the atoms are required for us at the moment?

(In reply to Andreas Tolfsen (:ato) from comment #1)
> A while back I landed a patch adding a build target for Marionette atoms to
> Selenium.  Unfortunately I'm unsure if I've picked the right atom targets:
> 
>    
> https://code.google.com/p/selenium/source/
> detail?r=2f47941a58fb7bec448f6492a3d2b478c2f0a8c4
> 
> An initial test I did with the newly generated atoms.js file from `./go
> marionette:atoms` against the Marionette unit test suite showed no
> regressions.  

what is ./go marionette:atoms?


> However, I'm not familiar with the Gecko .jar packaging
> system.  If I replace testing/marionette/atoms/atoms.js and run `mach
> build`, will this trigger the right paths in the dependency graph to rebuild
> Firefox properly?

If you update the atoms in http://mxr.mozilla.org/mozilla-central/source/testing/marionette/atoms/atoms.js for the ones that need updating.

If you do |mach build| it will take care of everything for you

Comment 4

[Andreas Tolfsen ❲:ato❳]

(In reply to David Burns :automatedtester from comment #3)
> What changes in the atoms are required for us at the moment?
> 
> (In reply to Andreas Tolfsen (:ato) from comment #1)
> > A while back I landed a patch adding a build target for Marionette atoms to
> > Selenium.  Unfortunately I'm unsure if I've picked the right atom targets:
> >    
> > https://code.google.com/p/selenium/source/detail?r=2f47941a58fb7bec448f6492a3d2b478c2f0a8c4
> 
> what is ./go marionette:atoms?

It's a build target in Selenium for compiling the JavaScript atoms needed by
Marionette.  The file can then be copied from the output directory to the Mozilla
repo.    

> > However, I'm not familiar with the Gecko .jar packaging
> > system.  If I replace testing/marionette/atoms/atoms.js and run `mach
> > build`, will this trigger the right paths in the dependency graph to rebuild
> > Firefox properly?
> 
> If you update the atoms in
> http://mxr.mozilla.org/mozilla-central/source/testing/marionette/atoms/atoms.
> js for the ones that need updating.
> 
> If you do |mach build| it will take care of everything for you

Thanks, but do you know if I've picked the right atoms from Selenium?  Since they
get truncated and have probably changed a bit since they were put in Marionette
it's almost impossible for me to tell if they are the right ones.

Since you say that I should trust mach build for picking up the file for packaging
and the tests are passing, I guess it's safe to assume they are indeed the right
ones and that I can make a patch with the upgraded atoms?

Comment 5

[David Burns :automatedtester]

(In reply to Andreas Tolfsen (:ato) from comment #4)

> Thanks, but do you know if I've picked the right atoms from Selenium?  

Since I have never seen that build target I would check that they are pulling the same atoms in as what is in atoms.js

Comment 6

[Andreas Tolfsen ❲:ato❳]

Attachment: 0001-Bug-964738-Upgrade-Marionette-atoms-to-Selenium-2.39.patch

try is closed at the moment, so unable to trigger testrun.  I suggest holding off with the review until we've been able to run all tests with these new atoms in place.

Comment 7

[Andreas Tolfsen ❲:ato❳]

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

Comment 8

[David Burns :automatedtester]

Mnw had failed but think its intermittents, have retriggered those tests

Comment 9

[Andreas Tolfsen ❲:ato❳]

Where can I see the test results of the retriggered tests?

Comment 10

[David Burns :automatedtester]

click on the tests and then bottom left has the logs for that test.

Comment 11

[Andreas Tolfsen ❲:ato❳]

Attempting another try: https://tbpl.mozilla.org/?tree=Try&rev=383de5e68eec

Comment 12

[Andreas Tolfsen ❲:ato❳]

Giving it one more try because Gu has been known to be unstable recently: https://tbpl.mozilla.org/?tree=Try&rev=17e324428561

Comment 13

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8367435 [details] [diff] [review]
0001-Bug-964738-Upgrade-Marionette-atoms-to-Selenium-2.39.patch

Need to investigate Mnw failures, dropping review request.

Comment 14

[Andreas Tolfsen ❲:ato❳]

We will not upgrade the Selenium atoms but remove them in their entirety as we will implement the W3C specification instead.