Readd integration tests for notification lang/direction in gaia

RESOLVED FIXED

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

unspecified
x86_64
Linux

Firefox Tracking Flags

(b2g-v1.2 fixed)

Details

Attachments

(2 attachments, 4 obsolete attachments)

46 bytes, patch
mikehenrty
: review+
Details | Diff | Splinter Review
46 bytes, patch
mikehenrty
: review+
Details | Diff | Splinter Review
Due to some conflict issues between 915002 and 899574, lang/dir integration tests for 915002 got stomped. Readd tests.

Blocking because it should've been part of 915002, which blocked koi+.
Created attachment 818112 [details] [review]
Patch 1 (v1) - Add lang/dir integration tests for notifications

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 915002
[User impact] if declined: Less testing
[Testing completed]: This IS tests.
[Risk to taking this patch] (and alternatives if risky): None
[String changes made]: None
Attachment #818112 - Flags: review?(mhenretty)
Attachment #818112 - Flags: approval-gaia-v1.2?(anygregor)
blocking-b2g: koi? → ---
Comment on attachment 818112 [details] [review]
Patch 1 (v1) - Add lang/dir integration tests for notifications

test-only. Doesn't need approval.
Attachment #818112 - Flags: approval-gaia-v1.2?(anygregor)
https://hg.mozilla.org/mozilla-central/rev/041bb1ec91ab
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Merge was accidental, Subsequently backed out
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 818112 [details] [review]
Patch 1 (v1) - Add lang/dir integration tests for notifications

Comment's on github. I don't think we are going to uplift Notification.get() (bug 899574) to 1.2, so these tests can't get uplifted either since they use that feature.
Created attachment 819250 [details] [diff] [review]
Patch 2 (v1) - Gaia v1.2 - Add lang/dir integration tests for notifications
Attachment #819250 - Flags: review?(mhenretty)
Created attachment 819255 [details] [review]
Patch 1 (v2) - Add lang/dir integration tests for notifications

Updated to address review comments
Attachment #818112 - Attachment is obsolete: true
Attachment #818112 - Flags: review?(mhenretty)
Attachment #819255 - Flags: review?(mhenretty)
Comment on attachment 819255 [details] [review]
Patch 1 (v2) - Add lang/dir integration tests for notifications

LGTM
Attachment #819255 - Flags: review?(mhenretty) → review+
Comment on attachment 819250 [details] [diff] [review]
Patch 2 (v1) - Gaia v1.2 - Add lang/dir integration tests for notifications

Review of attachment 819250 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: apps/system/test/marionette/notification_test.js
@@ +1,4 @@
>  var assert = require('assert'),
> +    NotificationTest = require('./lib/notification').NotificationTest,
> +    NotificationList = require('./lib/notification').NotificationList,
> +    Marionette = require('marionette-client'),

Nit: don't think we use this.
Attachment #819250 - Flags: review?(mhenretty) → review+
Created attachment 824118 [details] [review]
Patch 1 (v3) - Add lang/dir integration tests for notifications

Asking for rereview because this changed pretty massively.
Attachment #819255 - Attachment is obsolete: true
Attachment #824118 - Flags: review?(mhenretty)
Created attachment 824179 [details] [diff] [review]
Patch 1 (v4) - Add lang/dir integration tests for notifications

Fixed nits and optional argument issues.
Attachment #824118 - Attachment is obsolete: true
Attachment #824118 - Flags: review?(mhenretty)
Attachment #824179 - Flags: review?(mhenretty)
Comment on attachment 824179 [details] [diff] [review]
Patch 1 (v4) - Add lang/dir integration tests for notifications

LGTM. Let's wait for Travis to go green and land.
Attachment #824179 - Flags: review?(mhenretty) → review+
Patch 1 Landed to master

https://github.com/qdot/gaia/commit/ab40624fc6947b8cb7eed68bac09e6ecbdc285ef
Duplicate of this bug: 930772
Created attachment 824398 [details] [diff] [review]
Patch 2 (v2) - Gaia v1.2 - Add lang/dir integration tests for notifications

Rereview of the tests for v1.2. Only real difference between m-c and this is the persisting of the notification by hanging on wrappedJSObject.
Attachment #819250 - Attachment is obsolete: true
Attachment #824398 - Flags: review?(mhenretty)
Attachment #824398 - Flags: review?(mhenretty) → review+
v1.2: https://github.com/mozilla-b2g/gaia/commit/063a22e36a0641970176206e1ed3998202e14d37
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
status-b2g-v1.2: --- → fixed
You need to log in before you can comment on or make changes to this bug.