[marionette-content-script] Support new Service notification topic

RESOLVED FIXED

Status

Testing Graveyard
JSMarionette
RESOLVED FIXED
4 years ago
3 months ago

People

(Reporter: jugglinmike, Assigned: jugglinmike)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
In bug 935793, some service notification topics are changing. This work is blocked by existing Marionette tests because the current implementation of `marionette-content-script` only operates with the currently-implemented topic set. Update `marionette-content-script` to support *both* topic sets so that bug 935793 may be safely landed without triggering downstream errors.
(Assignee)

Updated

4 years ago
Blocks: 952177
(Assignee)

Comment 1

4 years ago
Created attachment 8350173 [details] [review]
Pull request on GitHub.com

Hi Gareth,

This patch is meant to support the proposed upstream change to b2g described in bug 935793. Some background: that patch landed in Mozilla Central and was backed out a day later. This is fortunate for us, because we have a easy way to test :P

1. Download the build of b2g intended for your system: http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/2013-12-15-04-02-01-mozilla-central/
2. Move or remove any existing `b2g/` directory from the root of the `marionette-content-script` project
3. Place the downloaded `b2g` directory in the project root
4. Run `make test` and watch it fail
5. Apply this patch
6. Run `make test` and watch it pass

...for thoroughness, you can also replace the original `b2g/` directory and run the tests again (since this change is intended to function with both versions).
Attachment #8350173 - Flags: review?(gaye)
Hey Mike - Sorry for the delay! I'll review this week.
review ping for :gaye - I'd really like to get bug 935793 landed!
Comment on attachment 8350173 [details] [review]
Pull request on GitHub.com

Sorry for the review delay and thank you for your thoroughness Mike!
Attachment #8350173 - Flags: review?(gaye) → review+
jugglinmike: is there any issues blocking this Marionette fix? markh's e10s tests are blocked waiting on this fix.
Flags: needinfo?(mike)
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 6

4 years ago
master: https://github.com/mozilla-b2g/marionette-content-script/commit/ec7fba3abc6f75befce28022bcb8d18657f7a8e1

Chris: sorry for the delay--I lost the e-mail about Gareth's approval in a deluge of Gaia GitHub.com notifications.

Hey Evan: I've updated the module's `package.json` for a new patch release [1], but I don't have publish rights on NPM. Could you publish 0.0.3 so this patch can be enjoyed downstream? Any maybe give me publish rights while you're at it? Thanks!

[1] https://github.com/mozilla-b2g/marionette-content-script/commit/aa1aabafd6e54c4612f67bae9a8b25caf65d7f04
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(mike) → needinfo?(evanxd)
Resolution: --- → FIXED
(Assignee)

Comment 7

4 years ago
Evan: I got in touch with James and I can now publish a new version. (At least, I have the permissions to do so--NPM is experiencing some sort of overage at the moment and won't respond to my request.)

I'll publish this as soon as I'm able!
Flags: needinfo?(evanxd)
(Assignee)

Comment 8

4 years ago
Okay, this patch is now available via npm at version 0.0.3 of the module:

https://www.npmjs.org/package/marionette-content-script

Chris: I believe this is all contingent on Gaia receiving the fix. Are you able to update Gaia's `package.json` file to reference this latest release? If not, let me know with another "needsinfo" flag and I will do so.
Flags: needinfo?(cpeterson)
Thanks, Mike!

Mark: do you know who should update Gaia's package.json file?
Flags: needinfo?(cpeterson) → needinfo?(mhammond)
Sorry, I've no idea about updating Gaia, so back to Mike
Flags: needinfo?(mhammond) → needinfo?(mike)
(Assignee)

Comment 11

4 years ago
master: https://github.com/mozilla-b2g/gaia/commit/f8b283c7384b373352b5aa94eb399dc4a88374be

Sorry for the delay!
Flags: needinfo?(mike)

Updated

3 months ago
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.