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

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gwagner, Assigned: gerard-majax)

Tracking

unspecified
mozilla38
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment, 12 obsolete attachments)

7.18 KB, patch
ahal
: review+
gerard-majax
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
(Reporter)

Updated

4 years ago
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)
(Reporter)

Comment 2

4 years ago
Alex, any idea why this is failing?
Flags: needinfo?(anygregor)
(Assignee)

Updated

4 years ago
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 ;)
(Assignee)

Comment 5

4 years ago
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
(Assignee)

Comment 6

4 years ago
Created attachment 8559104 [details] [diff] [review]
Detect Mulet when running workers tests r=bent
(Assignee)

Comment 8

4 years ago
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.
(Assignee)

Comment 9

4 years ago
Created attachment 8559133 [details] [diff] [review]
Detect Mulet when running workers tests r=bent
(Assignee)

Updated

4 years ago
Attachment #8559104 - Attachment is obsolete: true
(Assignee)

Comment 10

4 years ago
Created attachment 8559164 [details] [diff] [review]
Detect Mulet when running workers tests r=bent
(Assignee)

Updated

4 years ago
Attachment #8559133 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8559164 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8559164 - Attachment is obsolete: false
(Assignee)

Comment 12

4 years ago
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.
(Assignee)

Comment 13

4 years ago
(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
(Assignee)

Comment 14

4 years ago
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)
(Assignee)

Comment 15

4 years ago
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.
(Assignee)

Comment 16

4 years ago
Created attachment 8559209 [details] [diff] [review]
Detect Mulet when running workers tests r=bent
(Assignee)

Updated

4 years ago
Attachment #8559164 - Attachment is obsolete: true
(Assignee)

Comment 17

4 years ago
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)
(Assignee)

Comment 19

4 years ago
Created attachment 8559242 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent
(Assignee)

Updated

4 years ago
Attachment #8559209 - Attachment is obsolete: true
(Assignee)

Comment 20

4 years ago
You're right, and after testing locally, it seems to be doing the job :)
(Assignee)

Comment 22

4 years ago
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 :(
(Assignee)

Updated

4 years ago
Depends on: 1072443
(Assignee)

Comment 23

4 years ago
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)
(Assignee)

Comment 24

4 years ago
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)
(Assignee)

Comment 25

4 years ago
Created attachment 8559385 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent,ahal
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))
(Assignee)

Updated

4 years ago
Attachment #8559385 - Attachment is obsolete: true
(Assignee)

Comment 27

4 years ago
Created attachment 8559399 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent,ahal
(Assignee)

Updated

4 years ago
Attachment #8559399 - Attachment is obsolete: true
(Assignee)

Comment 29

4 years ago
Created attachment 8559404 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent,ahal
(Assignee)

Updated

4 years ago
Attachment #8559404 - Attachment is obsolete: true
(Assignee)

Comment 31

4 years ago
Created attachment 8559664 [details] [diff] [review]
Detect Mulet for Worker Interfaces tests, and disable Intl for B2G r=bent,ahal
(Assignee)

Updated

4 years ago
Blocks: 1129849
(Assignee)

Comment 33

4 years ago
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.
(Assignee)

Updated

4 years ago
Attachment #8559664 - Flags: review?(bent.mozilla)
Attachment #8559664 - Flags: review?(ahalberstadt)
(Assignee)

Comment 34

4 years ago
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)
(Assignee)

Comment 35

4 years ago
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)
(Assignee)

Comment 36

4 years ago
Created attachment 8559734 [details]
Hacking device_id for exposing "B2G" substring on B2G Desktop/Mulet.

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)
(Assignee)

Comment 38

4 years ago
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)
(Assignee)

Comment 39

4 years ago
ok, sorry, my B2G Desktop UA was WRONG, the real one is: Mozilla/5.0 (rv:38.0) Gecko/20100101 Firefox/38.0
(Assignee)

Comment 40

4 years ago
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)
(Assignee)

Comment 41

4 years ago
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.
(Assignee)

Updated

4 years ago
Attachment #8559664 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8559734 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Depends on: 1130287
(Reporter)

Updated

4 years ago
Whiteboard: [systemsfe]
(Assignee)

Updated

4 years ago
Flags: needinfo?(jonas)
(Assignee)

Comment 42

4 years ago
Created attachment 8561299 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

Make use of SpecialPowers.isB2G to do the proper detection, and fix
mochitest execution for Mulet to include some B2G-specific prefs.
(Assignee)

Comment 43

4 years ago
This should fix this test. It should also trigger a new failure, which is already documented in bug 1129849.
(Assignee)

Comment 45

4 years ago
Created attachment 8561336 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

Make use of SpecialPowers.isB2G to do the proper detection, and fix
mochitest execution for Mulet to include some B2G-specific prefs.
(Assignee)

Updated

4 years ago
Attachment #8561299 - Attachment is obsolete: true
(Assignee)

Comment 47

4 years ago
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+
(Assignee)

Comment 49

4 years ago
Sure, the fix was suggested by someone working on this, but I'll add him as a reviewer.
(Assignee)

Comment 50

4 years ago
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+
(Assignee)

Comment 53

4 years ago
Created attachment 8562106 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

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+
(Assignee)

Comment 55

4 years ago
Comment on attachment 8562106 [details] [diff] [review]
Properly detect B2G for workers interface tests r=bent

Carrying r+ from :bent
Attachment #8562106 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #8561336 - Attachment is obsolete: true
(Assignee)

Comment 57

4 years ago
try is green, and dependency bug has landed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1b3b467f1df5
Status: NEW → RESOLVED
Last Resolved: 4 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.