Closed Bug 1401129 Opened 2 years ago Closed 2 years ago

Release geckodriver 0.20.0

Categories

(Testing :: geckodriver, enhancement, P1)

enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: whimboo, Assigned: ato)

References

Details

(Keywords: meta)

Attachments

(8 files, 3 obsolete files)

59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
59 bytes, text/x-review-board-request
whimboo
: review+
Details
Filing this bug as a meta bug so that we can track which bugs should be fixed for the 0.20 release.
Depends on: 1401131
Depends on: 1403510
Depends on: 1403923
Depends on: 1406006
Depends on: 1406132
Depends on: 1410838
Depends on: 1411026
Depends on: 1413292
Depends on: 1414254
Depends on: 1421766
Depends on: 1430123
Depends on: 1429511
Depends on: 1430152
Depends on: 1430157
Depends on: 1430158
Depends on: 1431058
No longer depends on: 1431058
Depends on: 1431459
No longer depends on: 1431459
Depends on: 1433495
Depends on: 1399158
Depends on: 1436830
Depends on: 1437561
Depends on: 1437570
Depends on: 1437571
Priority: -- → P3
Version: 57 Branch → unspecified
No longer depends on: 1437570
Depends on: 1396819
Depends on: 1439329
No longer depends on: 1401131, 1403510, 1406006, 1410838, 1421766, 1433495
As discussed in our meeting today we want to get a new release out. It means that any still not fixed dependencies have been moved to the 0.21.0 release now.
Assignee: nobody → ato
Priority: P3 → P1
(In reply to Andreas Tolfsen ‹:ato› from bug 1433133 comment #26)
> I am resurrecting browser.newtabpage.introShown as part of
> https://bugzilla.mozilla.org/show_bug.cgi?id=1401129.

Note that that pref hasn't existed since Firefox 46 (bug 1229636). I'm still curious how old a copy of Firefox is meant to work with this code, and your comment in the cset here says "release" versions - what does that mean? Just release, or also ESR? Previous ESR? Firefox 4.0 ? Firefox 1.0 ?
Flags: needinfo?(ato)
At the time of writing geckodriver targets Firefox 55 and greater
[1].  If browser.newtabpage.introShown was removed in Firefox 46
it is safe to not resurrect it here and we can drop the patch.

  [1]
  https://searchfox.org/mozilla-central/source/testing/geckodriver/README.md#supported-firefoxen
Flags: needinfo?(ato)
Comment on attachment 8957186 [details]
Bug 1401129 - Restore datareporting.healthreport.about.reportUrl in automation tools.

https://reviewboard.mozilla.org/r/226126/#review232060

r=me with comment addressed.

::: testing/marionette/client/marionette_driver/geckoinstance.py:36
(Diff revision 1)
>          # and causing false-positive test failures. See bug 1176798, bug 1177018,
>          # bug 1210465.
>          "apz.content_response_timeout": 60000,
>  
>          # Do not send Firefox health reports to the production server
> +        "datareporting.healthreport.about.reportUrl": "http://%(server)s/dummy/abouthealthreport/",  # removed in Firefeox 59

It was also removed from `testing/marionette/server.js`.
Attachment #8957186 - Flags: review?(hskupin) → review+
Comment on attachment 8957186 [details]
Bug 1401129 - Restore datareporting.healthreport.about.reportUrl in automation tools.

https://reviewboard.mozilla.org/r/226126/#review232060

> It was also removed from `testing/marionette/server.js`.

We don’t need to restore it to Marionette because it ships with Firefox.
Comment on attachment 8957187 [details]
Bug 1401129 - Restore extensions.shield-recipe-client.api_url in automation tools.

https://reviewboard.mozilla.org/r/226128/#review232066

r=me with comment addressed.

::: testing/marionette/client/marionette_driver/geckoinstance.py:57
(Diff revision 1)
>          # Disable metadata caching for installed add-ons by default
>          "extensions.getAddons.cache.enabled": False,
>          # Disable intalling any distribution add-ons
>          "extensions.installDistroAddons": False,
>          # Make sure Shield doesn't hit the network.
>          "app.normandy.api_url": "",

I would prefer moving this up to keep the alphabetical order of preferences. This applies to all three files.

Maybe move both up to the new location and mark the extension pref as being replaced in Firefox 60.
Attachment #8957187 - Flags: review?(hskupin) → review+
Comment on attachment 8957188 [details]
Bug 1401129 - Restore browser.newtabpage.introShown for automation tools.

https://reviewboard.mozilla.org/r/226130/#review232070

r=me with comments addressed.

::: testing/geckodriver/src/prefs.rs:44
(Diff revision 1)
> +        // Assume the about:newtab pages intro panels have been shown
> +        // to not depend on which test runs first and happens to open
> +        // about:newtab.
> +        //
> +        // Removed in Firefox 60.
> +        ("browser.newtabpage.introShown", Pref::new(true)),

Please move it before `browser.offline` in all cases. Also note that it has been removed from `testing/marionette/components/marionette.js`.

::: testing/marionette/client/marionette_driver/geckoinstance.py:508
(Diff revision 1)
>          "browser.newtabpage.enabled": False,
>  
> +        # Assume the about:newtab page's intro panels have been shown to not depend on
> +        # which test runs first and happens to open about:newtab.
> +        #
> +        # Removed in Firefox 60.        

trailing slashes.
Attachment #8957188 - Flags: review?(hskupin) → review+
Comment on attachment 8957189 [details]
Bug 1401129 - Add peer review note to automation files.

https://reviewboard.mozilla.org/r/226132/#review232074

r=me with comment addressed.

::: testing/marionette/client/marionette_driver/geckoinstance.py:25
(Diff revision 1)
>  from six import reraise
>  
>  from . import errors
>  
>  
> +# ALL CHANGES TO THIS FILE MUST HAVE REVIEW FROM A MARIONETTE PEER!

This also needs to be added to `testing/marionette/components/marionette.js`.
Attachment #8957189 - Flags: review?(hskupin) → review+
Comment on attachment 8957190 [details]
Bug 1401129 - Release mozversion 0.1.3.

https://reviewboard.mozilla.org/r/226134/#review232078
Attachment #8957190 - Flags: review?(hskupin) → review+
Comment on attachment 8957191 [details]
Bug 1401129 - Release mozrunner 0.6.0.

https://reviewboard.mozilla.org/r/226136/#review232080

::: commit-message-11755:5
(Diff revision 1)
> +Bug 1401129 - Release mozrunner 0.5.1. r?whimboo
> +
> +Only change was a bump of the log dependency, which warrants a
> +point release.
> +

Bug 1431459 is part of this release which added the builder API to mozrunner. So I think it needs to be `0.6`.
Attachment #8957191 - Flags: review?(hskupin) → review-
Comment on attachment 8957192 [details]
Bug 1401129 - Release webdriver 0.35.0.

https://reviewboard.mozilla.org/r/226138/#review232082

r=me with comments addressed.

::: Cargo.lock:695
(Diff revision 1)
>   "clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)",
>   "hyper 0.10.13 (registry+https://github.com/rust-lang/crates.io-index)",
>   "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
>   "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
>   "mozprofile 0.3.0",
>   "mozrunner 0.5.1",

Please do not forget to update this line if we release it as `0.6` now.

::: testing/webdriver/Cargo.toml
(Diff revision 1)
>  [package]
>  name = "webdriver"
> -version = "0.34.0"
> +version = "0.35.0"
> -authors = ["Mozilla <tools-marionette@lists.mozilla.org>"]

Why did you remove this line?
Attachment #8957192 - Flags: review?(hskupin) → review+
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

r=me with comments addressed.

::: testing/geckodriver/CHANGES.md:26
(Diff revision 1)
> -  Marionette stacktraces
> +  Marionette stacktraces.
> -
> -- `Delete Session` now allows Firefox to safely shutdown within 70s before
> -  force-killing the process
>  
> -- Changed preference used to disable shield studies to `app.normandy.api_url`.
> +- [webdriver crate] upgraded to 0.35.0.

We also listed mozrunner crate upgrades for former releases, and so we should proceed with.

::: testing/geckodriver/CHANGES.md:861
(Diff revision 1)
>  [Jason Juang]: https://github.com/juangj
>  [Joshua Bruning]: https://github.com/joshbruning
>  [Kalpesh Krishna]: https://github.com/martiansideofthemoon
>  [Mike Pennisi]: https://github.com/jugglinmike
>  [Sven Jost]: https://github/mythsunwind
>  [Vlad Filippov]: https://github.com/vladikoff

Do we want to add Greg Fraley here, or for what is this section used? Some references aren't used at all in this file.
Attachment #8957193 - Flags: review?(hskupin) → review+
Comment on attachment 8957194 [details]
Bug 1401129 - Release geckodriver 0.20.0.

https://reviewboard.mozilla.org/r/226142/#review232090

r=me with comment addressed.

::: testing/geckodriver/Cargo.toml
(Diff revision 1)
>  [package]
>  name = "geckodriver"
> -version = "0.19.1"
> +version = "0.20.0"
> -authors = ["Mozilla <tools-marionette@lists.mozilla.org>"]

Why did you remove this line?
Attachment #8957194 - Flags: review?(hskupin) → review+
Comment on attachment 8957189 [details]
Bug 1401129 - Add peer review note to automation files.

https://reviewboard.mozilla.org/r/226132/#review232074

> This also needs to be added to `testing/marionette/components/marionette.js`.

We don’t need it in Marionette because it ships with Firefox,
so the preferences in that file should reflect the reality of Firefox.
Comment on attachment 8957192 [details]
Bug 1401129 - Release webdriver 0.35.0.

https://reviewboard.mozilla.org/r/226138/#review232082

> Why did you remove this line?

I don’t think we need this.
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

> We also listed mozrunner crate upgrades for former releases, and so we should proceed with.

I don’t think this is useful because they are built from in tree.

> Do we want to add Greg Fraley here, or for what is this section used? Some references aren't used at all in this file.

I don’t know his GitHub handle.
Comment on attachment 8957194 [details]
Bug 1401129 - Release geckodriver 0.20.0.

https://reviewboard.mozilla.org/r/226142/#review232090

> Why did you remove this line?

We don’t publish geckodriver so it is not needed.
Attachment #8957185 - Attachment is obsolete: true
Attachment #8957185 - Flags: review?(hskupin)
Attachment #8957188 - Attachment is obsolete: true
Attachment #8957190 - Attachment is obsolete: true
Comment on attachment 8957189 [details]
Bug 1401129 - Add peer review note to automation files.

https://reviewboard.mozilla.org/r/226132/#review232074

> We don’t need it in Marionette because it ships with Firefox,
> so the preferences in that file should reflect the reality of Firefox.

Oh, you are right. Completely missed that.
Comment on attachment 8957192 [details]
Bug 1401129 - Release webdriver 0.35.0.

https://reviewboard.mozilla.org/r/226138/#review232082

> I don’t think we need this.

Each crate needs a list of authors. Isn't that a mandatory field? It should really be left, so Mozilla is named for the crate on crates.io.
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

> I don’t think this is useful because they are built from in tree.

I don't see why it makes a difference in where a dependent crate is located at?

> I don’t know his GitHub handle.

I see.
Comment on attachment 8957191 [details]
Bug 1401129 - Release mozrunner 0.6.0.

https://reviewboard.mozilla.org/r/226136/#review232266

r=me with comment addressed.

::: testing/mozbase/rust/mozrunner/Cargo.toml
(Diff revision 2)
>  [package]
>  name = "mozrunner"
> -version = "0.5.0"
> +version = "0.6.0"
> -authors = ["Mozilla Tools and Automation <auto-tools@mozilla.com>"]

Leave this in for every crate.
Attachment #8957191 - Flags: review?(hskupin) → review+
Depends on: 1444431
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

> I don't see why it makes a difference in where a dependent crate is located at?

These utility crates aren’t relevant to the function of geckodriver,
e.g. doesn’t impact the contract of what it provides.  The webdriver
crate _is relevant_ because any change to its user-facing types or
interfaces will have a direct impact on users of the geckodriver
API.

It can be argued that even the webdriver crate should be left out
of the change log since every (relevant) change made to it is echoed
here, but I think it is useful to announce that a particular version
of geckodriver was built using a particular version of webdriver,
since it is such an instrumental part to providing a WebDriver
interface if you are writing an implementation in Rust.  This can
be useful to other consumers of the webdriver crate, such as Servo.
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

> These utility crates aren’t relevant to the function of geckodriver,
> e.g. doesn’t impact the contract of what it provides.  The webdriver
> crate _is relevant_ because any change to its user-facing types or
> interfaces will have a direct impact on users of the geckodriver
> API.
> 
> It can be argued that even the webdriver crate should be left out
> of the change log since every (relevant) change made to it is echoed
> here, but I think it is useful to announce that a particular version
> of geckodriver was built using a particular version of webdriver,
> since it is such an instrumental part to providing a WebDriver
> interface if you are writing an implementation in Rust.  This can
> be useful to other consumers of the webdriver crate, such as Servo.

In the same vein, we don’t include other upgraded crates it depends
on, partly because it is also not relevant but mainly because crate
upgrades can happen at any time in central for any reason, and it
would be impractical to find out which have changed.
No longer depends on: 1444431
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

> In the same vein, we don’t include other upgraded crates it depends
> on, partly because it is also not relevant but mainly because crate
> upgrades can happen at any time in central for any reason, and it
> would be impractical to find out which have changed.

Ok, so if we have something important to add from one of our crates, we could still list it in the geckodriver changelog. Sounds fine.
Comment on attachment 8957186 [details]
Bug 1401129 - Restore datareporting.healthreport.about.reportUrl in automation tools.

https://reviewboard.mozilla.org/r/226126/#review232060

> We don’t need to restore it to Marionette because it ships with Firefox.

But not with the value we expect to have when starting Firefox via `-marionette`. It would only have our expected value when used via marionette client, or geckodriver.
Comment on attachment 8957194 [details]
Bug 1401129 - Release geckodriver 0.20.0.

https://reviewboard.mozilla.org/r/226142/#review232090

> We don’t publish geckodriver so it is not needed.

We don't publish any of our in-tree Rust packages neither, but for every crate it is set.
Comment on attachment 8957193 [details]
Bug 1401129 - Note changes for geckodriver 0.20.0 release.

https://reviewboard.mozilla.org/r/226140/#review232084

> Ok, so if we have something important to add from one of our crates, we could still list it in the geckodriver changelog. Sounds fine.

Right, yes.  I definitely do think it makes sense to highlight
changes in user-noticable behaviour in depedent crate.
Comment on attachment 8957194 [details]
Bug 1401129 - Release geckodriver 0.20.0.

https://reviewboard.mozilla.org/r/226142/#review232090

> We don't publish any of our in-tree Rust packages neither, but for every crate it is set.

We do actually publish most crates (mozrunner, webdriver,
mozprofile, mozversion), so I’ve reverted the earlier changes to
their Cargo.toml files.

geckodriver isn’t published to crates.io, and in fact we disallow
publishing to crates.io with the publish = false option.  So in this
concrete case I don’t see any reason this line should be there.
Comment on attachment 8957186 [details]
Bug 1401129 - Restore datareporting.healthreport.about.reportUrl in automation tools.

https://reviewboard.mozilla.org/r/226126/#review232060

> But not with the value we expect to have when starting Firefox via `-marionette`. It would only have our expected value when used via marionette client, or geckodriver.

And why does that matter?  We don’t maintain Firefox
version-specific preference lists.  It’s been removed in Firefox 59,
so it will have no effect.
Comment on attachment 8957194 [details]
Bug 1401129 - Release geckodriver 0.20.0.

https://reviewboard.mozilla.org/r/226142/#review232090

> We do actually publish most crates (mozrunner, webdriver,
> mozprofile, mozversion), so I’ve reverted the earlier changes to
> their Cargo.toml files.
> 
> geckodriver isn’t published to crates.io, and in fact we disallow
> publishing to crates.io with the publish = false option.  So in this
> concrete case I don’t see any reason this line should be there.

If that is the rule of thumb, then it's fine with me.
Comment on attachment 8957186 [details]
Bug 1401129 - Restore datareporting.healthreport.about.reportUrl in automation tools.

https://reviewboard.mozilla.org/r/226126/#review232060

> And why does that matter?  We don’t maintain Firefox
> version-specific preference lists.  It’s been removed in Firefox 59,
> so it will have no effect.

Ok then.
Attachment #8957907 - Flags: review?(ato)
Comment on attachment 8957907 [details]
Bug 1401129 - Release mozversion 0.1.3.

https://reviewboard.mozilla.org/r/226868/#review232616
Attachment #8957907 - Flags: review?(ato) → review+
Comment on attachment 8957907 [details]
Bug 1401129 - Release mozversion 0.1.3.

https://reviewboard.mozilla.org/r/226868/#review232752
Attachment #8957907 - Flags: review?(hskupin) → review+
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/99dd9271e788
Restore datareporting.healthreport.about.reportUrl in automation tools. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/b5e15e66b86a
Restore extensions.shield-recipe-client.api_url in automation tools. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/95793c4be21c
Add peer review note to automation files. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/32c9b97fc10b
Release mozversion 0.1.3. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/313304bfbfa5
Release mozrunner 0.6.0. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/eadd632eefb0
Release webdriver 0.35.0. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/7a1e77dd53f6
Note changes for geckodriver 0.20.0 release. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/62453a779f13
Release geckodriver 0.20.0. r=whimboo
Blocks: 1444922
You need to log in before you can comment on or make changes to this bug.