Closed Bug 1027242 Opened 5 years ago Closed 3 years ago

Green Mulet up

Categories

(Testing :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX
mozilla34

People

(Reporter: armenzg, Assigned: ochameau)

References

Details

Attachments

(2 files, 2 obsolete files)

No description provided.
So this patch does two things:

1) It exposes 'mulet' to *.ini manifest files.
I'm not sure that's the best option, but it allows to see precisely,
which tests currently running on Firefox desktop, don't run on mulet.
And on most of the cases, it disable tests that were already disabled on b2g desktop.
One alternative would be to tweak mozinfo.py in order to
ensure that buildapp=b2g on Mulet.
The benefit of the current patch is that, I think we are running way more test than on b2g desktop:
$ egrep -r "mulet" --include *.ini . | grep "skip-if" | wc -l
  28
$ egrep -r "buildapp == 'b2g'" --include *.ini . | grep "skip-if" | wc -l
  618

2) Modify all necessary *.ini in order to be green.

Fabrice, any opinion on filtering tests for mulet?
Should I just KISS and run only what is being run on b2g desktop?


Otherwise, here is the list of tests that are disabled on mulet, but not on b2g desktop:
  dom/indexedDB/test/unit/test_globalObjects_ipc.js
  content/canvas/test/webgl-mochitest/test_webgl_conformance.html
  js/xpconnect/tests/mochitest/test_bug384632.html
  dom/workers/test/test_navigator.html
We would need to look and verify why they can't run on mulet.
Attachment #8442391 - Flags: feedback?(ahalberstadt)
Attachment #8442391 - Flags: feedback?(fabrice)
Comment on attachment 8442391 [details] [diff] [review]
Blacklist mulet's failing tests + expose 'mulet' to ini files

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

::: dom/alarm/test/mochitest.ini
@@ +1,5 @@
>  [DEFAULT]
>  skip-if = e10s
>  
>  [test_alarm_add_data.html]
> +skip-if = ((mulet || buildapp == 'b2g') && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage

This works, but here mulet implies toolkit != gonk so I think it is more clear if it is outside both sets of brackets.

::: dom/permission/tests/mochitest.ini
@@ +6,5 @@
>  
>  [test_alarms.html]
>  [test_browser.html]
>  [test_embed-apps.html]
> +skip-if = ((mulet || buildapp == 'b2g') && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage

Same as above

::: python/mozbuild/mozbuild/mozinfo.py
@@ +82,5 @@
>      d['healthreport'] = substs.get('MOZ_SERVICES_HEALTHREPORT') == '1'
>      d['asan'] = substs.get('MOZ_ASAN') == '1'
>      d['tests_enabled'] = substs.get('ENABLE_TESTS') == "1"
>      d['bin_suffix'] = substs.get('BIN_SUFFIX', '')
> +    d['mulet'] = bool(substs.get('MOZ_MULET'))

I think this makes sense. What is the value of buildapp for mulet? I assume toolkit is the same as desktop firefox.
Attachment #8442391 - Flags: feedback?(ahalberstadt) → feedback+
Comment on attachment 8442391 [details] [diff] [review]
Blacklist mulet's failing tests + expose 'mulet' to ini files

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

lgtm, but I'm not the best reviewer there.
Attachment #8442391 - Flags: feedback?(fabrice) → feedback+
Comment on attachment 8442391 [details] [diff] [review]
Blacklist mulet's failing tests + expose 'mulet' to ini files

This seems to have stalled out a bit; ted, can you review?
Attachment #8442391 - Flags: review?(ted)
(In reply to Andrew Halberstadt [:ahal] from comment #2)
> Comment on attachment 8442391 [details] [diff] [review]
> ::: python/mozbuild/mozbuild/mozinfo.py
> @@ +82,5 @@
> >      d['healthreport'] = substs.get('MOZ_SERVICES_HEALTHREPORT') == '1'
> >      d['asan'] = substs.get('MOZ_ASAN') == '1'
> >      d['tests_enabled'] = substs.get('ENABLE_TESTS') == "1"
> >      d['bin_suffix'] = substs.get('BIN_SUFFIX', '')
> > +    d['mulet'] = bool(substs.get('MOZ_MULET'))
> 
> I think this makes sense. What is the value of buildapp for mulet? I assume
> toolkit is the same as desktop firefox.

buildapp is b2g/dev for mulet. So that we end up ignoring all the "skip-if= buildapp == 'b2g'".
As said in comment 1, I can keep it simple and just make it so that buildapp is 'b2g' for mulet,
and keep running the exact same test set as b2g desktop.
We just have an opportunity to run 590 more tests... I'm not sure we really care about running all these additional tests, but I can see various tests being blacklisted in dom/ on b2g desktop, that can be run on Mulet.

One other alternative is to keep mozinfo.py as-is and filter with 'buildapp = b2g/dev' instead of 'mulet'.

I attached a detailed listing of all manifests being blacklisted on b2g desktop but not on mulet.
Assignee: nobody → poirot.alex
I'm going to land this preemptively on fig to see the effects there, as soon as the tree re-opens.
This patch didn't seem to have any effect, something must not be hooked quite correctly.
Comment on attachment 8442391 [details] [diff] [review]
Blacklist mulet's failing tests + expose 'mulet' to ini files

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

I don't have a strong opinion here, my only comment is that putting one more variable in the mix for the already-complicated b2g manifests is going to make it even harder to get these right. You've already got buildapp and toolkit to worry about. I guess if we're building mulet then it's a problem we have to face anyway, and 'mulet' is more self-descriptive than 'buildapp == "b2g/whatever"'.
Attachment #8442391 - Flags: review?(ted) → review+
(In reply to Jonathan Griffin (:jgriffin) from comment #7)
> This patch didn't seem to have any effect, something must not be hooked
> quite correctly.

The patch is fine, there are just a lot of other failures that need to be skipped as well.
Attached patch patch (obsolete) — Splinter Review
Another version of this patch, with more tests being blacklisted
and using "buildapp == 'mulet'" instead of just "mulet".

A try run: https://tbpl.mozilla.org/?tree=Try&rev=da3acc2395ef
Hopefully I got try syntax correctly. I used 'linux64-mulet'.
Attachment #8442391 - Attachment is obsolete: true
Looks like the mozinfo changes should be:

    if substs.get('MOZ_MULET') == "1":

instead of:

    if substs['MOZ_MULET'] == "1":
Depends on: 1033511
Attached patch patchSplinter Review
Update with more tests being blacklisted... some start to be green!
https://tbpl.mozilla.org/?tree=Try&rev=0b10db3dbded
Attachment #8448224 - Attachment is obsolete: true
I've landed this on fig, along with the change from comment #11.
Depends on: 1024183
Depends on: 1036415
Depends on: 1039834
Depends on: 1039840
Depends on: 1040927
Depends on: 1040975
This work is still progressing on fig.  mochitest-plain is very close, browser-chrome and devtools pretty close.  Lots of work to do yet with mochitest-chrome.

As soon as I have a working suite, I'll file a bug to schedule across all trunk branches.
(In reply to Jonathan Griffin (:jgriffin) from comment #15)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/4f6d9db92389

This gets us to green for mochitest-plain at least.
For some weird reason, this patch caused us to re-enable a previously disabled Android test which then failed on TBPL.

Backed out as https://hg.mozilla.org/integration/mozilla-inbound/rev/8710fe40a825, and then landed a fixed version as https://hg.mozilla.org/integration/mozilla-inbound/rev/64586374a208
I don't know why this patch would have affected OSX 10.6...maybe a bad merge at some point.  Here's a try run to make sure we don't break anything else:

https://tbpl.mozilla.org/?tree=Try&rev=3d5ad03340c3
(In reply to Jonathan Griffin (:jgriffin) from comment #19)
> I don't know why this patch would have affected OSX 10.6...maybe a bad merge
> at some point.  Here's a try run to make sure we don't break anything else:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=3d5ad03340c3

Possibly this change? https://hg.mozilla.org/integration/mozilla-inbound/rev/64586374a208#l130.1
(In reply to Wes Kocher (:KWierso) from comment #20)
> (In reply to Jonathan Griffin (:jgriffin) from comment #19)
> > I don't know why this patch would have affected OSX 10.6...maybe a bad merge
> > at some point.  Here's a try run to make sure we don't break anything else:
> > 
> > https://tbpl.mozilla.org/?tree=Try&rev=3d5ad03340c3
> 
> Possibly this change?
> https://hg.mozilla.org/integration/mozilla-inbound/rev/64586374a208#l130.1

Yes, you're right, thanks.
Depends on: 1047572
Depends on: 1047571
Depends on: 1048441
Depends on: 1128924
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.