Closed Bug 1129173 Opened 10 years ago Closed 10 years ago

Mulet permafail: dom/workers/test/test_worker_interfaces.html | false: Intl should be defined on the global scope - expected PASS

Categories

(Core :: DOM: Workers, defect)

Product:
Core
Component:
DOM: Workers
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla38
Tracking Flags:
Tracking Status
firefox38 --- fixed

People

(Reporter: gwagner, Assigned: gerard-majax)

References

Details

(Whiteboard: [systemsfe])

Attachments

(1 file, 12 obsolete files)

Properly detect B2G for workers interface tests r=bent
(deleted), patch
ahal
: review+
gerard-majax
: review+
Details | Diff | Splinter Review 
      No description provided.
Blocks: 1129164
This seems like something that could break Gaia.  You should probably figure this one out rather than disable the test.
Flags: needinfo?(anygregor)
Alex, any idea why this is failing?
Flags: needinfo?(anygregor)
Assignee: nobody → lissyx+mozillians
This is the same thing than bug 1128924 comment 2.
I think the other Alex is ready to fix tham all ;)
The current status is that Mulet is detected as:
> ----- createInterfaceMap: isNightly=true -----
> ----- createInterfaceMap: isRelease=false -----
> ----- createInterfaceMap: isDesktop=true -----
> ----- createInterfaceMap: isB2G=false -----

This gives the following failures:
> INFO TEST-UNEXPECTED-FAIL | dom/workers/test/test_worker_interfaces.html | false: Intl should  be defined on the global scope - expected PASS
> INFO TEST-UNEXPECTED-FAIL | dom/workers/test/test_worker_interfaces.html | 1 === 0: The following interface(s) are not enumerated: Intl - expected PASS

When forcing isB2G, to true, we get:
> INFO TEST-UNEXPECTED-FAIL | dom/workers/test/test_worker_interfaces.html | false: Intl should  be defined on the global scope - expected PASS
> INFO TEST-UNEXPECTED-FAIL | dom/workers/test/test_worker_interfaces.html | false: DataStore should  be defined on the global scope - expected PASS
> INFO TEST-UNEXPECTED-FAIL | dom/workers/test/test_worker_interfaces.html | false: DataStoreCursor should  be defined on the global scope - expected PASS
> INFO TEST-UNEXPECTED-FAIL | dom/workers/test/test_worker_interfaces.html | 3 === 0: The following interface(s) are not enumerated: Intl, DataStore, DataStoreCursor - expected PASS
In the configure.in file, http://mxr.mozilla.org/mozilla-central/source/configure.in#8997
> 8995 if test "$MOZ_WIDGET_TOOLKIT" = "android" ||
> 8996    test "$MOZ_BUILD_APP" = "b2g" ||
> 8997    test "$MOZ_BUILD_APP" = "b2g/dev"; then
> 8998     _INTL_API=no
> 8999 else
> 9000     _INTL_API=yes
> 9001 fi

So Intl API is being explicitely disabled on Mulet, as on B2G.
Attachment #8559104 - Attachment is obsolete: true
Attachment #8559133 - Attachment is obsolete: true
Attachment #8559164 - Attachment is obsolete: true
Attachment #8559164 - Attachment is obsolete: false
So we should have DataStore on Mulet (since they are expected on B2G), but it looks like we really don't. The previous try will be green but that's not good. I'll keep the 'mulet' field in entry because we need to disable for |Intl| case, but it looks like we should also fix the lack of DataStore in workers.
(In reply to Alexandre LISSY :gerard-majax from comment #12)
> So we should have DataStore on Mulet (since they are expected on B2G), but
> it looks like we really don't. The previous try will be green but that's not
> good. I'll keep the 'mulet' field in entry because we need to disable for
> |Intl| case, but it looks like we should also fix the lack of DataStore in
> workers.

Ok, so on my Mulet it fails because of |status == nsIPrincipal::APP_STATUS_CERTIFIED| at https://dxr.mozilla.org/mozilla-central/source/dom/datastore/DataStoreService.cpp#1135
And from what I see, status = 0, i.e. APP_STATUS_NOT_INSTALLED. Which may be totally obvious, in the case of a mochitest ... :)

Ben do you have any great idea for this?
Flags: needinfo?(bent.mozilla)
In testing/profiles/prefs_b2g_unittest.js, the pref dom.testing.datastore_enabled_for_hosted_apps is being defined to true. When I run mochitest locally, I issue:
> ./mach mochitest-plain dom/workers/test/test_worker_interfaces.html

So it's possible that, indeed, it's already all okay on try.
Attachment #8559164 - Attachment is obsolete: true
And the matching try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c65e2504dd32

This one should have DataStore working as expected according to comment 15, and it should have Intl disabled properly on Mulet to match configure.in as explained in comment 8.
Rather than have Intl have an explicit entry for "mulet: false" wouldn't it be better to make that "b2g: false"? IMO you shouldn't need to ever have something specific to mulet because then mulet !== b2g.
Flags: needinfo?(bent.mozilla)
Attachment #8559209 - Attachment is obsolete: true
You're right, and after testing locally, it seems to be doing the job :)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c65e2504dd32&exclusion_state=all Mochitest 4 fails because of datastore. It means the prefs from testing/profiles/prefs_b2g_unittest.js are included :(
Depends on: 1072443
Comment on attachment 8559242 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent

Mochitests 4 are now failing because DataStore lacks the proper pref to be able to work from mochitest, probably because of bug 1072443.
Attachment #8559242 - Flags: review?(bent.mozilla)
Comment on attachment 8559242 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent

According to :ahal on IRC, fixing bug 1072443 may not be trivial, but we can hack a bit for now.
Attachment #8559242 - Attachment is obsolete: true
Attachment #8559242 - Flags: review?(bent.mozilla)
Comment on attachment 8559385 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent,ahal

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

::: testing/mochitest/runtests.py
@@ +1207,5 @@
> +       preferences += [ os.path.join(SCRIPT_DIR, 'profile_data', 'prefs_b2g_unittest.js') ]
> +
> +    for prefsPath in preferences:
> +      prefs = dict(Preferences.read_prefs(prefsPath))
> +      prefs.update(self.extraPrefs(options.extraPrefs))

The last prefsPath in the for loop will overwrite all the others, you'll want it to look like this:

prefs = {}
for path in preferences:
    prefs.update(Preferences.read_prefs(path))
prefs.update(self.extraPrefs(options.extraPrefs))
Attachment #8559385 - Attachment is obsolete: true
Attachment #8559399 - Attachment is obsolete: true
Attachment #8559404 - Attachment is obsolete: true
So, we have Mochitest 4 in better shape for mulet: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6a17046a0da&exclusion_state=all

The new issue is expected and tracked in bug 1129849.
Attachment #8559664 - Flags: review?(bent.mozilla)
Attachment #8559664 - Flags: review?(ahalberstadt)
Or maybe we could make use of bug 1072050: on B2G we can have an optional "device id" part in the user agent. Since a lot of the tests relies on the user agent to detect B2G, would it may make sense to make use of it ?

i.e., have "B2G" or "Mulet" substring in the useragent to make the detection easier and more reliable ?
Flags: needinfo?(khuey)
Flags: needinfo?(jonas)
Flags: needinfo?(bent.mozilla)
Comment on attachment 8559664 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent,ahal

Hold back review, waiting for the needinfo to be addressed, since it may simplify a lot this.
Attachment #8559664 - Flags: review?(bent.mozilla)
Attachment #8559664 - Flags: review?(ahalberstadt)
This is what it would look like. And on mochitests, we would just:
> var isB2G = navigator.userAgent.contains("B2G");
Hrm, can we not just override the useragent entirely to look exactly like stock b2g? Adding "B2G" in the device id part is something unique that a real b2g build would never do, right?
Flags: needinfo?(bent.mozilla)
I'll look into this solution. FTR, the user agents are:

Mulet :
=======
UserAgent :: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0

Browser :
=========
UserAgent :: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0

B2G Desktop :
=============
UserAgent :: Mozilla/5.0 (B2G; rv:38.0) Gecko/20100101 Firefox/38.0
Flags: needinfo?(khuey)
Flags: needinfo?(jonas)
ok, sorry, my B2G Desktop UA was WRONG, the real one is: Mozilla/5.0 (rv:38.0) Gecko/20100101 Firefox/38.0
Fabrice suggested to use SpecialPowers for this. Currently, a lot of tests are using the user agent string. Jonas, what's your opinion, which solution suits you best?
Flags: needinfo?(jonas)
Ok, or another solution is making B2G Desktop showing "Mobile" in the user agent, as it should be expected. Currently, this is only done when we have "FXOS_SIMULATOR" defined.
Attachment #8559664 - Attachment is obsolete: true
Attachment #8559734 - Attachment is obsolete: true
Depends on: 1130287
Whiteboard: [systemsfe]
Flags: needinfo?(jonas)
Make use of SpecialPowers.isB2G to do the proper detection, and fix
mochitest execution for Mulet to include some B2G-specific prefs.
This should fix this test. It should also trigger a new failure, which is already documented in bug 1129849.
Make use of SpecialPowers.isB2G to do the proper detection, and fix
mochitest execution for Mulet to include some B2G-specific prefs.
Attachment #8561299 - Attachment is obsolete: true
Comment on attachment 8561336 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

Looks greenish. I had to add a field for excluding android also for Intl otherwise it was breaking mochitests on android 4.0. Ben, if you see a better way, feel free :)
Attachment #8561336 - Flags: review?(bent.mozilla)
Comment on attachment 8561336 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

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

This looks ok to me, but you should flag someone else for the runtests.py change. It looks fine, but they may have a better idea of how testing prefs should be glued together. Joel Maher maybe?
Attachment #8561336 - Flags: review?(bent.mozilla) → review+
Sure, the fix was suggested by someone working on this, but I'll add him as a reviewer.
Comment on attachment 8561336 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

Andrew, we need your review for the python changes :)
Attachment #8561336 - Flags: review?(ahalberstadt)
Comment on attachment 8561336 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

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

Looks good, but r- until question in second comment is cleared up.

::: testing/mochitest/runtests.py
@@ +1205,5 @@
> +    preferences = [ os.path.join(SCRIPT_DIR, 'profile_data', 'prefs_general.js') ]
> +
> +    # TODO: Let's include those prefs until bug 1072443 is fixed
> +    if mozinfo.info.get('buildapp') == 'mulet':
> +       preferences += [ os.path.join(SCRIPT_DIR, 'profile_data', 'prefs_b2g_unittest.js') ]

This is fine, but could just do preferences.append(os.path.join(...))

@@ +1215,5 @@
>      prefs.update(self.extraPrefs(options.extraPrefs))
>  
>      # interpolate preferences
> +    interpolation = {"server": "%s:%s" % (options.webServer, options.httpPort),
> +                     "OOP": "false" } # TODO: Remove OOP once bug 1072443 is fixed

Shouldn't this be behind an "if mulet"? Or do you actually mean to disable OOP on all b2g mochitest jobs, including emulators?
Attachment #8561336 - Flags: review?(ahalberstadt) → review-
Comment on attachment 8561336 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

:gerard-majax pointed out that this is in the desktop harness and the OOP key won't have any affect on how that runs. I'd still like to see it behind an "if mulet" just to make it clear that it is unrelated to desktop Firefox, so I'll change this to r+ with nits.
Attachment #8561336 - Flags: review- → review+
Make use of SpecialPowers.isB2G to do the proper detection, and fix
mochitest execution for Mulet to include some B2G-specific prefs.
Comment on attachment 8562106 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

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

Thanks!
Attachment #8562106 - Flags: review+
Comment on attachment 8562106 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

Carrying r+ from :bent
Attachment #8562106 - Flags: review+
Attachment #8561336 - Attachment is obsolete: true
try is green, and dependency bug has landed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1b3b467f1df5
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: