Closed
Bug 909036
Opened 11 years ago
Closed 11 years ago
Fix test_sms_basics.html for b2g
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 908439
People
(Reporter: martijn.martijn, Assigned: martijn.martijn)
References
()
Details
Attachments
(1 file)
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
This is the usual change to pushPrefEnv and pushPermissions. But I also had to change some other stuff.
This test file made the assumption that the 'dom.sms.enabled' pref and the 'sms' permission was enabled.
If it isn't, (and on b2g mochitest, that's the case), this causes failures.
The 'dom.sms.enabled' pref and the 'sms' permission are only applied after a page reload, so a lot of what this test file is doing seems superfluous or it should reload the iframe after every pref/permission change.
What do you think, Vicamo?
This test file is also disabled on Android, btw.
Attachment #795089 -
Flags: review?(vyang)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → martijn.martijn
Comment 1•11 years ago
|
||
We should also remove the line in testing/mochitest/b2g.json if this patch fixes this bug, shouldn't we? Trying locally.
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
I don't see the removal of test test_sms_basics.html line in the tryserver patch, I don't think it worked.
But I can include it in the tryserver that I'm going to pull up for bug 908439.
Do you think my patch is ok? Like I said, the test could perhaps be made much simpler, perhaps.
Comment 4•11 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #3)
> I don't see the removal of test test_sms_basics.html line in the tryserver
> patch, I don't think it worked.
It's in https://hg.mozilla.org/try/rev/e62a7b92413a#l2.12
And you can find the successful test results in mochitest-4 (https://tbpl.mozilla.org/php/getParsedLog.php?id=27056792&tree=Try).
> But I can include it in the tryserver that I'm going to pull up for bug
> 908439.
> Do you think my patch is ok? Like I said, the test could perhaps be made
> much simpler, perhaps.
That's just fine.
Comment 5•11 years ago
|
||
Comment on attachment 795089 [details] [diff] [review]
test_sms_basics.diff
Review of attachment 795089 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you!
Attachment #795089 -
Flags: review?(vyang) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Vicamo, thanks for your help, but I'm going to include this patch as part of bug 908439, it's easier for me that way.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•