Closed Bug 909036 Opened 11 years ago Closed 11 years ago

Fix test_sms_basics.html for b2g

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 908439

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

References

()

Details

Attachments

(1 file)

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: nobody → martijn.martijn
We should also remove the line in testing/mochitest/b2g.json if this patch fixes this bug, shouldn't we?  Trying locally.
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.
(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 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+
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.

Attachment

General

Created:
Updated:
Size: