Closed Bug 1369709 Opened 7 years ago Closed 7 years ago

Release geckodriver 0.17.0

Categories

(Testing :: geckodriver, enhancement)

Product:
Testing
Component:
geckodriver
Version:
Version 3
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file)

Bug 1369709 - Release geckodriver 0.17.0.
59 bytes, text/x-review-board-request
ato
: review+
Details 
We need a new release of geckodriver to fix a severe bug (bug 1364385) for users on Windows. As a side-along we can also include bug 1369708.
Whiteboard: [geckodriver]
Depends on: 1369827
Component: Marionette → geckodriver
Whiteboard: [geckodriver]
How do we want to continue the CHANGES.md file? Should it still contain all the details or would the originally used commit messages be fine? I find it a very time-intensive task to continue it as it was used.
Flags: needinfo?(dburns)
Flags: needinfo?(ato)
(In reply to Henrik Skupin (:whimboo) from comment #1)
> How do we want to continue the CHANGES.md file? Should it still contain all
> the details or would the originally used commit messages be fine?

I think we should.  As you can tell from
https://github.com/mozilla/geckodriver/releases,
it has been immensely useful to track changes across versions.

It is a manually edited file and does not contain information from all commit
messages.  It also tracks changes across geckodriver, mozrunner, mozprofile,
and webdriver crates.

> I find it a very time-intensive task to continue it as it was used.

This is part of good software maintenance.
Flags: needinfo?(dburns)
Flags: needinfo?(ato)
Yes, we do need to maintain a Changes document.
Depends on: 1370597
Ok, I will take care about all the details.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
No longer depends on: 1369827
Here the changes so far extracted from the mozilla-central repository:

% hg log -M -r ec3f63b961b0:: --template "{desc|firstline}\n" .
geckodriver: Updated .travis.yml for linux32 optimized compile
geckodriver: Add extension commands for addon install, uninstall (#711)
geckodriver: marionette: correct error when there is no current session (#701)
geckodriver: Make trace logs safe for windows prompt (#722)
Bug 1366728 - Allow Window Rect dimensions to pass through after Set Window Rect; r=jgraham
Bug 1340637 - Provide build rules for geckodriver; r=jgraham,ted
Bug 1340637 - Update geckodriver cargo lockfile for vendored crates; r=ted
Bug 1341102 - Revendor rust dependencies again after several manual update on a CLOSED TREE.
Bug 1341102 - Update vendored clap to get more consistent bitflags on a CLOSED TREE.
Bug 1364385 - Do not use canonicalized path to start Firefox. r=ato
Bug 1369708 - Avoid setting of default preferences by mozrunner. r=ato

I will use that as base for updating the CHANGES.md file, and will leave out those which only affects our build config and such.
Lets also get the fullscreen support included which should also land soon via bug 1370510.
Depends on: 1370510
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review150880

::: commit-message-4dd1d:1
(Diff revision 1)
> +Bug 1369709 - Release geckodriver 0.16.2.

Maybe it should be 0.17, given that we have API additions and some changes?
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review150880

> Maybe it should be 0.17, given that we have API additions and some changes?

Yes, I agree.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

::: testing/geckodriver/CHANGES.md:10
(Diff revision 1)
> +  - POST `/session/{sessionId}/moz/addon/install` to install an extension [Gecko only]
> +  - POST `/session/{sessionId}/moz/addon/uninstall` to uninstall an extension [Gecko only]

s/sessionId/session id/g

::: testing/geckodriver/CHANGES.md:10
(Diff revision 1)
> +  - POST `/session/{sessionId}/moz/addon/install` to install an extension [Gecko only]
> +  - POST `/session/{sessionId}/moz/addon/uninstall` to uninstall an extension [Gecko only]

What does “[Gecko only]” mean?

::: testing/geckodriver/CHANGES.md:17
(Diff revision 1)
> +
> +### Changed
> +- Increasing the length of the `network.http.phishy-userpass-length` preference will cause Firefox to not prompt when navigating to a website with a username or password in the URL
> +- Library dependencies upgraded to mozrunner 0.4 and mozprofile 0.3 to allow overriding of preferences via capabilities if those have been already set in the profile
> +- Library dependencies upgraded to mozversion 0.1.2 to only use the normalized path of the Firefox binary for version checks but not to actually start the browser, which broke several components in Firefox on Windows
> +- Use ASCII versions of array symbols to properly display them in the Windows command prompt

I could nominate this as a fix rather than a change.

::: testing/geckodriver/CHANGES.md:20
(Diff revision 1)
> +- Library dependencies upgraded to mozrunner 0.4 and mozprofile 0.3 to allow overriding of preferences via capabilities if those have been already set in the profile
> +- Library dependencies upgraded to mozversion 0.1.2 to only use the normalized path of the Firefox binary for version checks but not to actually start the browser, which broke several components in Firefox on Windows
> +- Use ASCII versions of array symbols to properly display them in the Windows command prompt
> +
> +### Fixed
> +- Allow Window Rect dimensions to pass through after Set Window Rect

I don’t understand what is fixed.

::: testing/geckodriver/CHANGES.md:21
(Diff revision 1)
> +- Library dependencies upgraded to mozversion 0.1.2 to only use the normalized path of the Firefox binary for version checks but not to actually start the browser, which broke several components in Firefox on Windows
> +- Use ASCII versions of array symbols to properly display them in the Windows command prompt
> +
> +### Fixed
> +- Allow Window Rect dimensions to pass through after Set Window Rect
> +- Use "SessionNotCreated" error instead of "UnknownError" if there is no current session

s/SessionNotCreated/‘session not created’/
s/UnknownError/‘unknown error’/

… or link to the webdriver crate docs types.
Attachment #8875390 - Flags: review?(ato) → review-
Summary: Release geckodriver 0.16.2 → Release geckodriver 0.17.0
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

> What does “[Gecko only]” mean?

As you know this is not part of the webdriver spec and custom endpoints for Gecko only. How should we spell that?

> I don’t understand what is fixed.

That's from https://hg.mozilla.org/mozilla-central/rev/48baf93a251e. What would you like to see here?

If we don't make it too hard for the person to update the CHANGES.md file it might be good if patches to geckodriver automatically add their entries into the file? And not that we have to do it at the very end of a release cycle.

> s/SessionNotCreated/‘session not created’/
> s/UnknownError/‘unknown error’/
> 
> … or link to the webdriver crate docs types.

doc types is what we should use here to keep the same style as used for previous releases.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151486

::: testing/geckodriver/Cargo.lock:3
(Diff revision 2)
>  [root]
>  name = "geckodriver"
> -version = "0.16.1"
> +version = "0.16.2"

Something is broken with mozreview... I clearly have the correct version locally:

--- a/testing/geckodriver/Cargo.lock
+++ b/testing/geckodriver/Cargo.lock
@@ -1,6 +1,6 @@
 [root]
 name = "geckodriver"
-version = "0.16.1"
+version = "0.17.0"

A push shows:

no changes found
submitting 1 changesets for review
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151486

> Something is broken with mozreview... I clearly have the correct version locally:
> 
> --- a/testing/geckodriver/Cargo.lock
> +++ b/testing/geckodriver/Cargo.lock
> @@ -1,6 +1,6 @@
>  [root]
>  name = "geckodriver"
> -version = "0.16.1"
> +version = "0.17.0"
> 
> A push shows:
> 
> no changes found
> submitting 1 changesets for review

Hm, a no-op push fixed that? Interesting.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

> As you know this is not part of the webdriver spec and custom endpoints for Gecko only. How should we spell that?

Although it is somewhat implied by the /moz/ namespace that is Gecko-only, I think this makes sense when you explain it.  Dropping this.

> doc types is what we should use here to keep the same style as used for previous releases.

OK, I guess this is fine.  I couldn’t find any linkable types in the webdriver crate’s generated documentation.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

> Although it is somewhat implied by the /moz/ namespace that is Gecko-only, I think this makes sense when you explain it.  Dropping this.

From the first view it's not that visible, and frankly I only noticed the `moz` part now!

> Dropping this.

I assume it means I can close this issue.

> OK, I guess this is fine.  I couldn’t find any linkable types in the webdriver crate’s generated documentation.

See my latest version of the patch. It has links to the types.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

> That's from https://hg.mozilla.org/mozilla-central/rev/48baf93a251e. What would you like to see here?
> 
> If we don't make it too hard for the person to update the CHANGES.md file it might be good if patches to geckodriver automatically add their entries into the file? And not that we have to do it at the very end of a release cycle.

I would say something along the lines of “The SetWindowRect command now
returns the WindowRect when it is done”.  (Although I spotted a bug with
that changeset you linked to: it uses the ElementRect rather than the
WindowRect…)

> If we don't make it too hard for the person to update the CHANGES.md file
> it might be good if patches to geckodriver automatically add their entries
> into the file?

Indeed.  I would prefer each change to come with an update to the changelog.
I usually dot hat myself.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

> I would say something along the lines of “The SetWindowRect command now
> returns the WindowRect when it is done”.  (Although I spotted a bug with
> that changeset you linked to: it uses the ElementRect rather than the
> WindowRect…)
> 
> > If we don't make it too hard for the person to update the CHANGES.md file
> > it might be good if patches to geckodriver automatically add their entries
> > into the file?
> 
> Indeed.  I would prefer each change to come with an update to the changelog.
> I usually dot hat myself.

> Although I spotted a bug with
that changeset you linked to: it uses the ElementRect rather than the
WindowRect…)

Is that somewhat critical so we have to get this fixed first?

> Indeed.  I would prefer each change to come with an update to the changelog.
I usually dot hat myself.

If we want to do that we should announce it so everyone knows who is reviewing geckodriver patches.
Depends on: 1371555
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151296

> > Although I spotted a bug with
> that changeset you linked to: it uses the ElementRect rather than the
> WindowRect…)
> 
> Is that somewhat critical so we have to get this fixed first?
> 
> > Indeed.  I would prefer each change to come with an update to the changelog.
> I usually dot hat myself.
> 
> If we want to do that we should announce it so everyone knows who is reviewing geckodriver patches.

>> Although I spotted a bug with that changeset you linked to: it uses
>> the ElementRect rather than the WindowRect…)
>
> Is that somewhat critical so we have to get this fixed first?

I don’t think it’s critical as it functionally produces the same
JSON Object.  We should be fine to release.
Comment on attachment 8875390 [details]
Bug 1369709 - Release geckodriver 0.17.0.

https://reviewboard.mozilla.org/r/146828/#review151750
Attachment #8875390 - Flags: review?(ato) → review+
No longer depends on: 1371555
Adding the `leave-open` keyword to keep the bug open until we finally released the binaries.

Andreas, can you take care of that? Thanks.
Flags: needinfo?(ato)
Keywords: leave-open
Release is now on GitHub.
Flags: needinfo?(ato)
Thanks Andreas. Also the email about the release went out. Looks like we are done.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: