Closed
Bug 1083347
Opened 10 years ago
Closed 10 years ago
Remove android*.json
Categories
(Testing :: Mochitest, defect)
Tracking
(firefox39 fixed)
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: gbrown, Assigned: vaibhav1994)
References
Details
(Whiteboard: [ateam_harness_work])
Attachments
(7 files, 19 obsolete files)
31.57 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
7.83 KB,
patch
|
vaibhav1994
:
review+
|
Details | Diff | Splinter Review |
6.87 KB,
patch
|
vaibhav1994
:
review+
|
Details | Diff | Splinter Review |
2.51 KB,
patch
|
armenzg
:
review+
|
Details | Diff | Splinter Review |
4.13 KB,
patch
|
armenzg
:
review+
|
Details | Diff | Splinter Review |
11.60 KB,
patch
|
jmaher
:
review+
gbrown
:
review+
|
Details | Diff | Splinter Review |
5.83 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
We use testing/mochitest/android.json, android23.json, and androidx86.json to differentiate between Android 4.0 / Android 2.3 / Android x86 mochitests. Instead, we should have appropriate annotations in the normal mochitest.ini manifests.
I think Android x86 can be identified as os: 'android' && processor: 'x86'.
Differentiating 2.3 and 4.0 is more difficult. See http://hg.mozilla.org/mozilla-central/annotate/62f0b771583c/testing/mochitest/runtestsremote.py#l642 for some prior work to provide android_version for manifests. This is added at test run time -- it actually checks the run-time environment to determine what version of android is in use on the test device. As I recall, this is problematic because manifest annotations are also evaluated at build time.
Assignee | ||
Comment 1•10 years ago
|
||
Ok, so lets tackle this in pieces. Since the manifest annotations are evaluated at build time, should I first start with making it run from test harness? I would appreciate if I get some pointers to start on this one.
Reporter | ||
Comment 2•10 years ago
|
||
If you want to start with something straight-forward, I would try to eliminate androidx86.json first: We should be able to use 'os' and 'processor' annotations in mochitest.ini.
I'm not 100% sure that there is an issue with using 'android_version'. Maybe it would be useful if you pushed a quick test to try, with just a few android_version instances -- does it break anything? does it work as intended?
Assignee | ||
Comment 3•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=102324d67d28 - A push to know the mozinfo.info config for android so that we can go forward with determining appropriate annotations in manifest.
Comment 4•10 years ago
|
||
On a positive note, as of bug 1074507 we no longer do any build-time test filtering! That means you should be able to use the android_version key with impunity, as long as it's implemented properly in each harness.
Assignee | ||
Comment 5•10 years ago
|
||
So I checked for x86 mozinfo: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/vaibhavmagarwal@gmail.com-102324d67d28/try-android-x86/fennec-36.0a1.en-US.android-i386.mozinfo.json
'os' and 'toolkit' == 'android' and the 'processor' == 'x86'.
So for x86.json annotation, I would personally prefer
skip-if = toolkit == 'android' && processor == 'x86' because in all annotations previously ( Bug 970925 ), we have used 'toolkit', so for the sake of being consistency.
Assignee | ||
Comment 6•10 years ago
|
||
Patch for androidx86.json
This contains entries that were straight-forward and there are still ~25 entries remaining in androidx86.json, which have to require careful changing to manifests.
Comment 7•10 years ago
|
||
thanks for this patch! Any chance you have a try push?
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #7)
> Any chance you have a try push?
We only run Android x86 xpcshell and robocop tests on trunk trees, so a push to Cedar might be more useful -- I can do that.
Reporter | ||
Comment 9•10 years ago
|
||
There is a problem -- this breaks builds on all platforms. See https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=89587e237cb4.
Traceback (most recent call last):
File "./config.status", line 968, in <module>
config_status(**args)
File "/builds/slave/ced-and-x86-000000000000000000/build/python/mozbuild/mozbuild/config_status.py", line 148, in config_status
summary = the_backend.consume(definitions)
File "/builds/slave/ced-and-x86-000000000000000000/build/python/mozbuild/mozbuild/backend/base.py", line 181, in consume
for obj in objs:
File "/builds/slave/ced-and-x86-000000000000000000/build/python/mozbuild/mozbuild/frontend/emitter.py", line 137, in emit
objs = list(self.emit_from_context(out))
File "/builds/slave/ced-and-x86-000000000000000000/build/python/mozbuild/mozbuild/frontend/emitter.py", line 783, in emit_from_context
for obj in self._process_test_manifest(context, info, path):
File "/builds/slave/ced-and-x86-000000000000000000/build/python/mozbuild/mozbuild/frontend/emitter.py", line 977, in _process_test_manifest
context)
mozbuild.frontend.reader.SandboxValidationError:
==============================
ERROR PROCESSING MOZBUILD FILE
==============================
The error occurred while processing the following file or one of the files it includes:
/builds/slave/ced-and-x86-000000000000000000/build/content/media/moz.build
The error occurred when validating the result of the execution. The reported error is:
Error processing test manifest file /builds/slave/ced-and-x86-000000000000000000/build/content/media/test/mochitest.ini: Traceback (most recent call last):
File "/builds/slave/ced-and-x86-000000000000000000/build/python/mozbuild/mozbuild/frontend/emitter.py", line 840, in _process_test_manifest
m = manifestparser.TestManifest(manifests=[path], strict=True)
File "/builds/slave/ced-and-x86-000000000000000000/build/testing/mozbase/manifestparser/manifestparser/manifestparser.py", line 416, in __init__
self.read(*manifests)
File "/builds/slave/ced-and-x86-000000000000000000/build/testing/mozbase/manifestparser/manifestparser/manifestparser.py", line 572, in read
self._read(here, filename, defaults)
File "/builds/slave/ced-and-x86-000000000000000000/build/testing/mozbase/manifestparser/manifestparser/manifestparser.py", line 467, in _read
sections = read_ini(fp=fp, variables=defaults, strict=self.strict)
File "/builds/slave/ced-and-x86-000000000000000000/build/testing/mozbase/manifestparser/manifestparser/manifestparser.py", line 364, in read_ini
assert key not in current_section
AssertionError
Assignee | ||
Comment 10•10 years ago
|
||
:gbrown, yes I was testing it out locally. Sorry I did not see your comment and hence did not reply to it (was busy debugging locally). I have attached the new patch that is now building locally for me.
Attachment #8509448 -
Attachment is obsolete: true
Reporter | ||
Comment 11•10 years ago
|
||
Comment on attachment 8509448 [details] [diff] [review]
androidx86.patch
Review of attachment 8509448 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/media/test/mochitest.ini
@@ +318,1 @@
> skip-if = buildapp == 'mulet' || os == 'win' # bug 894922
Is this valid -- having more than one skip-if for a test?
Attachment #8509448 -
Attachment is obsolete: false
Assignee | ||
Comment 12•10 years ago
|
||
Cedar push by :gbrown https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=8d185e7c78ee
Assignee | ||
Comment 13•10 years ago
|
||
Continuing annotations for the remaining entries in androidx86.json.
Just one entry for 'robocop' remaining after this in excludetests, as I am not sure of it. In this patch, I have removed the entries of which there are no files present and have not annotated the manifest separately for those tests which have:
skip-if = toolkit == 'android'
as they are skipped for all of android.
Attachment #8509448 -
Attachment is obsolete: true
Comment 14•10 years ago
|
||
Comment on attachment 8510242 [details] [diff] [review]
androidx86_continued.patch
Review of attachment 8510242 [details] [diff] [review]:
-----------------------------------------------------------------
just a drive by review, lots of great progress.
::: dom/canvas/test/webgl-mochitest.ini
@@ +1,2 @@
> [DEFAULT]
> +skip-if = (toolkit == 'android' && processor == 'x86') #bug 865443 - separate suite -- mochitest-gl
isn't this the case for all of android?
::: dom/devicestorage/ipc/mochitest.ini
@@ +1,3 @@
> [DEFAULT]
> skip-if = toolkit == 'android' || e10s #bug 781789 & bug 782275
> +skip-if = (toolkit == 'android' && processor == 'x86') #bug 781789 & bug 782275
we can't have >1 skip-if line per section
::: dom/devicestorage/test/mochitest.ini
@@ +1,3 @@
> [DEFAULT]
> skip-if = e10s # bug 781789 & bug 782275
> +skip-if = (toolkit == 'android' && processor == 'x86') #bug 781789 & bug 782275
need a single skip-if
Assignee | ||
Comment 15•10 years ago
|
||
:jmaher thanks for the review! I have corrected the nits.
:gbrown, could you push this patch to cedar? Thanks!
Attachment #8510242 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8509714 [details] [diff] [review]
androidx86.patch
The results for the push: https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=8d185e7c78ee is ok I think, although there are failing chunks, but they are also in a previous push: https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=b66067d8c820
:gbrown, are the results ok according to you? Also, it would be great if you could review the patch. Adding you as the reviewer.
Attachment #8509714 -
Flags: review?(gbrown)
Reporter | ||
Comment 17•10 years ago
|
||
Comment on attachment 8509714 [details] [diff] [review]
androidx86.patch
Review of attachment 8509714 [details] [diff] [review]:
-----------------------------------------------------------------
The Cedar results look okay to me. There are some pre-existing crashes, so we lose some information, but I don't think we should let that block us here.
::: content/media/test/mochitest.ini
@@ +315,3 @@
> [test_buffered.html]
> [test_bug448534.html]
> +skip-if = buildapp == 'mulet' || os == 'win' || (toolkit == 'android' && processor == 'x86') # bug 894922 #x86 only bug 914439
Is "# ... # ..." legal? I guess so, but I think I would prefer a , or ; or something like that. (There are a few of these.)
Attachment #8509714 -
Flags: review?(gbrown) → review+
Reporter | ||
Comment 18•10 years ago
|
||
Comment on attachment 8510283 [details] [diff] [review]
androidx86_continued.patch
Review of attachment 8510283 [details] [diff] [review]:
-----------------------------------------------------------------
I had a quick look here, just to give you some feedback...
::: content/base/test/chrome.ini
@@ +1,5 @@
> [DEFAULT]
>
> [test_bug357450.js]
> [test_copypaste.xul]
> +skip-if = (toolkit == 'android' && processor == 'x86') #bug 904183
Tests in chrome.ini are run in "mochitest-chrome" test jobs, which do not run on Android. I think test_copypaste.xul was either accidentally added to androidx86.json, or that test previously ran as a regular mochitest but is now a chrome mochitest. Regardless, I would not update chrome.ini files: If a test in android*.json is only in chrome.ini manifests, simply remove it from android*.json.
::: dom/canvas/test/webgl-mochitest.ini
@@ +1,2 @@
> [DEFAULT]
> +skip-if = toolkit == 'android' #bug 865443 - separate suite -- mochitest-gl
This seems wrong. I think this will skip webgl-mochitest/test-backbuffer-channels.html, etc, for all Android, and that's not part of dom/canvas/test/webgl-conformance.
Assignee | ||
Comment 19•10 years ago
|
||
:gbrown, thanks for pointing out the mistakes! Could you push this patch to cedar for verification? (Just base this patch *above* the previous patch to prevent bitrot)
Attachment #8510283 -
Attachment is obsolete: true
Reporter | ||
Comment 20•10 years ago
|
||
Assignee | ||
Comment 21•10 years ago
|
||
Patch to clean up android23.patch
Assignee | ||
Comment 22•10 years ago
|
||
The "robocop" entry is required to be removed in all the android*.json
:jmaher has suggested, that we should use a [subsuite] keyword by adding a [default] section here: http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/robocop.ini#1 , so that we do not run the tests normally but only when we specify --subsuite=robocop. Is this is the right way to go forward, and if yes, will it require changes in the mozharness scripts?
Reporter | ||
Comment 23•10 years ago
|
||
I don't understand why we need to treat robocop.ini specially. We have chrome.ini and browser.ini files that are not confused with regular mochitest manifests -- why is robocop.ini different? What happens if you just remove the robocop entry from the android*.json files?
Comment 24•10 years ago
|
||
right now we skip robocop in the android*.json file. For android builds we include robocop/ directory (which includes robocop.ini) in the /tests/ subfolder (tests.zip). Assuming we remove robocop from the list to exclude, we would run all the robocop tests as part of the plain tests instead of inside their own job.
The idea to use a subsuite would allow us to ignore all robocop jobs unless we specify --subsuite=robocop on the cli to the python test harness. If I am overlooking something, do speak up, making this easier would be nice!
Reporter | ||
Comment 25•10 years ago
|
||
The mochitest manifest handling code is too complicated for me to say with any certainty. I'll try to understand this better with an experiment: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=76a9e587375f
Assignee | ||
Comment 26•10 years ago
|
||
Comment on attachment 8510439 [details] [diff] [review]
androidx86_continued.patch
:gbrown, the try results for this patch look good to me. Adding you as the reviewer for this patch.
Also, from the try push done for robocop, it looks like there is no problem caused by removing robocop entry from android*.json without annotating the manifests, though there maybe some hidden problem.
Attachment #8510439 -
Flags: review?(gbrown)
Assignee | ||
Comment 27•10 years ago
|
||
Comment on attachment 8510470 [details] [diff] [review]
android23.patch
:gbrown, kindly review this patch too and push this to cedar. Apologies for filling up your review queue :)
Attachment #8510470 -
Flags: review?(gbrown)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → vaibhavmagarwal
Assignee | ||
Comment 28•10 years ago
|
||
Please checkin androidx86.json, it is a r+
Keywords: checkin-needed
Comment 29•10 years ago
|
||
Reporter | ||
Comment 30•10 years ago
|
||
Comment on attachment 8510439 [details] [diff] [review]
androidx86_continued.patch
Review of attachment 8510439 [details] [diff] [review]:
-----------------------------------------------------------------
I thought the Cedar run was okay, but I just noticed that mochitest-gl is not running any tests!
http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/cedar-android/1414085296/cedar_ubuntu64_vm_mobile_test-mochitest-gl-bm52-tests1-linux64-build3.txt.gz
16:23:28 INFO - 1 INFO TEST-UNEXPECTED-ERROR | SimpleTest/TestRunner.js | No checks actually run
16:23:28 INFO - TEST-INFO
16:23:28 INFO - 2 INFO TEST-START | Shutdown
16:23:28 INFO - 3 INFO Passed: 0
16:23:28 INFO - 4 INFO Failed: 0
16:23:28 INFO - 5 INFO Todo: 0
Everything else looks fine here.
Attachment #8510439 -
Flags: review?(gbrown) → review-
Reporter | ||
Comment 31•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #26)
> Also, from the try push done for robocop, it looks like there is no problem
> caused by removing robocop entry from android*.json without annotating the
> manifests, though there maybe some hidden problem.
I think it is okay to simply remove the robocop entries from android*.json -- everything looks okay to me.
Reporter | ||
Comment 32•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #27)
> Comment on attachment 8510470 [details] [diff] [review]
> android23.patch
>
> :gbrown, kindly review this patch too and push this to cedar. Apologies for
> filling up your review queue :)
https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=3c1115f0a0a0
This is not a problem -- keep those patches coming!!
Assignee | ||
Comment 33•10 years ago
|
||
Nice observation :gbrown! I think this was probably because I mistakenly did:
skip = toolkit == 'android' for webgl-conformance tests thus it is skipping for all jobs
I have now replaced it with:
skip-if = (toolkit == 'android' && processor == 'x86') || android_version == "10" || android_version == "15"
to skip for android.json, android23.json and androidx86.json
Kindly push this patch to cedar if you think the logic is fine (also removed the robocop entries in this patch from android8.json since the results are as expected for them)
Attachment #8510439 -
Attachment is obsolete: true
Reporter | ||
Comment 34•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #33)
> Created attachment 8511168 [details] [diff] [review]
> androidx86_continued.patch
>
> Nice observation :gbrown! I think this was probably because I mistakenly did:
>
> skip = toolkit == 'android' for webgl-conformance tests thus it is skipping
> for all jobs
>
> I have now replaced it with:
>
> skip-if = (toolkit == 'android' && processor == 'x86') || android_version ==
> "10" || android_version == "15"
>
> to skip for android.json, android23.json and androidx86.json
I don't see how that will help Android 2.3/4.0 mochitest-gl. I'm not sure what is going wrong actually, so maybe this is worth a try...but I'm skeptical. :)
> Kindly push this patch to cedar if you think the logic is fine (also removed
> the robocop entries in this patch from android8.json since the results are
> as expected for them)
Cedar should only be necessary for checking on the Android x86 mochitests. Cedar is less efficient than try in that it starts builds for all platforms by default. Would you mind pushing to try instead? When we think everything is sorted out, I'll push a final version to Cedar to double-check on x86.
Assignee | ||
Comment 35•10 years ago
|
||
I have left web-conformance entry in android*.json and removed all other entries in this patch, so that we can push this in the meanwhile we find a solution for web-conformance entry.
Try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=48615c239c33
Attachment #8511168 -
Attachment is obsolete: true
Reporter | ||
Comment 36•10 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #32)
> https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=3c1115f0a0a0
Oops -- all the Android 2.3 mochitests failed here!
Whiteboard: [fixed-in-fx-team][leave open] → [leave open]
Reporter | ||
Comment 38•10 years ago
|
||
Comment on attachment 8510470 [details] [diff] [review]
android23.patch
Review of attachment 8510470 [details] [diff] [review]:
-----------------------------------------------------------------
This *looks* okay to me, but failed on Cedar. r- pending a successful try run.
Attachment #8510470 -
Flags: review?(gbrown) → review-
Assignee | ||
Comment 39•10 years ago
|
||
Error seen in try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=48615c239c33
12:41:41 INFO - 10-24 12:35:41.882 I/GeckoDump( 2256): TEST-UNEXPECTED-FAIL: setup.js | error parsing http://mochi.test:8888/android.json (SyntaxError: JSON.parse: expected double-quoted property name at line 5 column 3 of the JSON data)
12:41:41 INFO - 10-24 12:35:41.882 W/GeckoConsole( 2256): [JavaScript Error: "SyntaxError: JSON.parse: expected double-quoted property name at line 5 column 3 of the JSON data" {file: "http://mochi.test:8888/manifestLibrary.js" line: 49}]
Forgot to remove a comma at the end in the *.json in androidx86_continued.patch.
A new try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=901eeb3462c2
Assignee | ||
Comment 40•10 years ago
|
||
Another push to try for removing android23.json: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=600649c4dec1
Assignee | ||
Comment 41•10 years ago
|
||
The android jobs are green in the try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=600649c4dec1 (ignore the bc jobs on linux, they are because of a patch that has now been backed out from mozilla inbound).
:gbrown, kindly push to cedar if you feel necessary. Adding you as the reviewer for this patch.
Attachment #8510470 -
Attachment is obsolete: true
Attachment #8513718 -
Flags: review?(gbrown)
Reporter | ||
Comment 42•10 years ago
|
||
Comment on attachment 8513718 [details] [diff] [review]
android23.patch
Review of attachment 8513718 [details] [diff] [review]:
-----------------------------------------------------------------
The try run looks good to me, except I am not sure about Android 2.3 mochitest-gl -- I re-triggered.
r+ with nit addressed and assuming those retries are okay.
::: docshell/test/navigation/mochitest.ini
@@ +43,5 @@
> skip-if = (buildapp == 'b2g' && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage
> [test_popup-navigates-children.html]
> skip-if = buildapp == 'b2g' # b2g(Needs multiple window.open support, also uses docshelltreenode) b2g-debug(Needs multiple window.open support, also uses docshelltreenode) b2g-desktop(Needs multiple window.open support, also uses docshelltreenode)
> [test_reserved.html]
> +skip-if = (buildapp == 'b2g' && (toolkit != 'gonk' || debug)) || android_version == "10" #too slow on Android 2.3 aws only; bug 1030403
nit -- It looks to me like nearly all existing skip-if operations use single quotes, rather than double quotes: ' vs ". I think you should use single quotes, to be consistent.
Attachment #8513718 -
Flags: review?(gbrown) → review+
Reporter | ||
Comment 43•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #41)
> :gbrown, kindly push to cedar if you feel necessary.
I don't think it is necessary since there is nothing x86-specific here.
Assignee | ||
Comment 44•10 years ago
|
||
Carrying forward the r+
Mochitest-gl job is passing in the try push, log: http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/vaibhavmagarwal@gmail.com-600649c4dec1/try-android/try_ubuntu64_vm_mobile_test-mochitest-gl-bm118-tests1-linux64-build509.txt.gz
Attachment #8513718 -
Attachment is obsolete: true
Attachment #8514009 -
Flags: review+
Assignee | ||
Comment 45•10 years ago
|
||
This patch is passing on try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=21663d609d81
:gbrown, please push to cedar to test for androidx86.
Attachment #8511216 -
Attachment is obsolete: true
Comment 46•10 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #42)
> nit -- It looks to me like nearly all existing skip-if operations use single
> quotes, rather than double quotes: ' vs ". I think you should use single
> quotes, to be consistent.
The manifest expression parser supports both single and double-quoted strings, I don't really know why (probably because both Python and JS do). We could narrow that down to just single-quoted strings to make things forcibly consistent.
Reporter | ||
Comment 47•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #45)
> :gbrown, please push to cedar to test for androidx86.
https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=61a1a25cdd84
Assignee | ||
Comment 48•10 years ago
|
||
The try push: https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&revision=61a1a25cdd84 results look good.
:gbrown, please review this patch.
Attachment #8514011 -
Attachment is obsolete: true
Attachment #8514781 -
Flags: review?(gbrown)
Reporter | ||
Comment 49•10 years ago
|
||
Comment on attachment 8514781 [details] [diff] [review]
androidx86_continued.patch
Review of attachment 8514781 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/mochitest/androidx86.json
@@ -15,5 @@
> - "dom/imptests/html/webgl": "WebGL",
> - "dom/imptests/webapps/WebStorage/tests/submissions/Infraware/test_storage_local_key.html": "bug 775227",
> - "dom/inputmethod": "Not supported on Android",
> - "dom/media/test/test_play_twice.html": "x86 only bug 914439",
> - "dom/media/test/test_played.html": "bug 751539",
It looks like some tests removed from here (test_played, test_play_twice) are not added to any manifest. Why is that?
Assignee | ||
Comment 50•10 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #49)
> ::: testing/mochitest/androidx86.json
> @@ -15,5 @@
> > - "dom/imptests/html/webgl": "WebGL",
> > - "dom/imptests/webapps/WebStorage/tests/submissions/Infraware/test_storage_local_key.html": "bug 775227",
> > - "dom/inputmethod": "Not supported on Android",
> > - "dom/media/test/test_play_twice.html": "x86 only bug 914439",
> > - "dom/media/test/test_played.html": "bug 751539",
>
> It looks like some tests removed from here (test_played, test_play_twice)
> are not added to any manifest. Why is that?
All these tests have "skip-if = toolkit == android" in the manifest. So I did not add it as it would become redundant.
Reporter | ||
Comment 51•10 years ago
|
||
Right - I see that now.
Reporter | ||
Updated•10 years ago
|
Attachment #8514781 -
Flags: review?(gbrown) → review+
Assignee | ||
Comment 52•10 years ago
|
||
Carrying forward the r+. Fixed for bitrot.
Attachment #8514781 -
Attachment is obsolete: true
Attachment #8515107 -
Flags: review+
Assignee | ||
Comment 53•10 years ago
|
||
Carrying forward the r+. Fixed for bitrot.
Attachment #8514009 -
Attachment is obsolete: true
Attachment #8515108 -
Flags: review+
Assignee | ||
Comment 54•10 years ago
|
||
Kindly checkin android23.patch and androidx86_continued.patch. Thanks
Keywords: checkin-needed
Comment 55•10 years ago
|
||
Keywords: checkin-needed
Comment 56•10 years ago
|
||
hi vaibhav, the 23patch didn't apply cleanly (i guess conflicted with the androidx86 patch.
renamed 1083347 -> android23.patch
applying android23.patch
patching file testing/mochitest/android23.json
Hunk #1 FAILED at 0
1 out of 1 hunks FAILED -- saving rejects to file testing/mochitest/android23.json.rej
could you take a look, thanks!
Flags: needinfo?(vaibhavmagarwal)
Comment 57•10 years ago
|
||
Assignee | ||
Comment 58•10 years ago
|
||
Fixed!
Attachment #8515108 -
Attachment is obsolete: true
Flags: needinfo?(vaibhavmagarwal)
Attachment #8516090 -
Flags: review+
Keywords: checkin-needed
Assignee | ||
Comment 62•10 years ago
|
||
Now, only 1 entry is left in android*.json: http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/androidx86.json#4
and we cannot simply remove this using something like 'skip-if = toolkit == android', as the android gl jobs run those tests: http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/gl.json#2
Afaik, we cannot differentiate between the android gl and normal android chunks. One of the solution can be to add another keyword to mozinfo for gl jobs like we do for android_version: http://hg.mozilla.org/mozilla-central/annotate/62f0b771583c/testing/mochitest/runtestsremote.py#l642
and then not skip for gl.
Other way might be to use a subsuite tag like we do for browser-chrome tests: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/layoutview/test/browser.ini#3
I am not sure what would be the correct way to fix this, and it would be great if someone could give ideas regarding fixing this!
Assignee | ||
Comment 63•10 years ago
|
||
:gbrown could you give some advice as to how I could go about fixing this?
Flags: needinfo?(gbrown)
Reporter | ||
Comment 64•10 years ago
|
||
I have not thought this out fully, but as a guideline, I would like the Android mochitest-gl suite to run the same way as it does on Linux, Mac, etc., if that is possible.
From test logs for mochitest-gl on other platforms, it looks like those other platforms use --manifest=mochitest-subsuite-webgl.ini instead of --test-manifest=gl.json. I don't immediately see how those tests are excluded from the main batch of mochitests though.
Bug 1064432 changed some things here -- that patch looks interesting.
Flags: needinfo?(gbrown)
Comment 65•10 years ago
|
||
we could push to try and test this. One way to do this without editing mozharness would be to:
* hack runtestsremote.py and if --test-manifest=='gl.json', set --manifest='mochitest-subsuite-webgl.ini' and any other differences
* remove gl.json from android*.json
I suspect there is one or two details in there that might need addressing, but within a few try pushes we could have a patch for m-c and mozharness to solve this. The problem is that we would need to get this on mozilla-aurora and mozilla-beta so the mozharness changes would work.
Lets start with a patch that hacks the changes we need in place, then we can figure out a deployment.
Reporter | ||
Comment 66•10 years ago
|
||
Recent changes in bug 1064002 might make deployment easier.
Comment 68•10 years ago
|
||
I landed the change to mozharness which will allow testing this on Try:
https://tbpl.mozilla.org/?tree=Ash&rev=6a32858f85ff
It needs to be merged to the production branch.
In any case, we can always re-trigger once it merges.
:vaibhav1994 feel free to land on try.
Comment 69•10 years ago
|
||
Trigger all Android unit test jobs. I want to review the jobs once you do so.
Comment 70•10 years ago
|
||
The mozharness code is now live.
You can push to try and let me know.
Assignee | ||
Comment 71•10 years ago
|
||
A test push to try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d94c8bca20e2
Comment 72•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #71)
> A test push to try:
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d94c8bca20e2
I re-triggered your jobs because the code run against the wrong revision of mozharness (my bad!).
Your re-triggered jobs will say:
https://hg.mozilla.org/build/mozharness/rev/4c021248ba40
Comment 73•10 years ago
|
||
This time it used the right mozharness code but it failed.
For the record, my change got backed out.
You will probably want to push to try by modifying testing/mozharness/mozharness.json and pointing it to my user repo (http://hg.mozilla.org/users/armenzg_mozilla.com/mozharness) until my code lands again.
Reporter | ||
Comment 74•10 years ago
|
||
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #73)
> This time it used the right mozharness code but it failed.
It looks like all the jobs tried to run all of the tests, constrained only by chunk: non-webgl tests ran even though --manifest was passed on the runtestsremote.py command line. Do we know why?
Comment 75•10 years ago
|
||
In bug 1064002 I discovered that if --test-manifest is not the last parameter we fail.
I'm trying to figure out what is needed to work around it as it is my last blocking issue.
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #86)
> It seems that for some harnesses the order of parameters can be troublesome.
> [1]
>
> This fails:
> --total-chunks=2 --test-manifest=gl.json --this-chunk=1
>
> This succeeds:
> --total-chunks=2 --this-chunk=1 --test-manifest=gl.json
>
> [1]
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=bd4f7fbc017b
> /builds/slave/test/build/venv/bin/python -u
> /builds/slave/test/build/tests/mochitest/runtestsremote.py --autorun
> --close-when-done --dm_trans=sut --console-level=INFO
> --app=org.mozilla.fennec --remote-webserver=10.0.2.2
> --xre-path=/builds/slave/test/build/hostutils/xre
> --utility-path=/builds/slave/test/build/hostutils/bin --deviceIP=127.0.0.1
> --devicePort=20701 --http-port=8854 --ssl-port=4454
> --certificate-path=/builds/slave/test/build/tests/certs
> --symbols-path=https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-
> builds/armenzg@mozilla.com-bd4f7fbc017b/try-android-api-9/fennec-37.0a1.en-
> US.android-arm.crashreporter-symbols.zip --quiet
> --log-raw=/builds/slave/test/build/blobber_upload_dir/raw_structured_logs.
> log --total-chunks=2 --test-manifest=gl.json --this-chunk=1
Assignee | ||
Comment 76•10 years ago
|
||
Hi! I got involved in other projects in auto-tools in the past month and this bug went unnoticed. I would like to finish up the remaining part in this bug. Previously, there were a lot of changes in mozharness happening, but now I think that they are probably done. By changing this line http://dxr.mozilla.org/mozilla-central/source/testing/config/mozharness/android_arm_config.py#34 and making it point to _webgl-conformance.ini does not yield the expected results, so I guess there might be some other way to do it. Armen, it would be really good if you could give me pointers about how to solve this piece of the puzzle. Thanks
Comment 77•10 years ago
|
||
Hi Vaibhav,
Happy new year!
Unfortunately, I got rather stuck in bug 1064002.
I will do new try pushes to see where we are. I've lost track of where we are.
I will NI myself to get back to you in the next 2 days. Please CC yourself to the bug.
Btw, can you push to try? or to my user repo? Once I know where we stand I might ask for your help if you would like to.
Depends on: 1064002
Flags: needinfo?(armenzg)
Assignee | ||
Comment 78•10 years ago
|
||
Armen, thanks for replying. I am ready to do try pushes to test things out, but I am not really sure what changes I have to make before pushing to try. We can discuss the changes here or I can ping you on IRC, whichever you feel comfortable.
Comment 79•10 years ago
|
||
I think it might have worked:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ecdb1c21ae76&filter-searchStr=gl1
I will double check.
Flags: needinfo?(armenzg)
Assignee | ||
Comment 80•10 years ago
|
||
The above try push has:
13:57:45 INFO - Running on test-1 the command /builds/slave/test/build/venv/bin/python -u /builds/slave/test/build/tests/mochitest/runtestsremote.py --autorun --close-when-done --dm_trans=sut --console-level=INFO --app=org.mozilla.fennec --remote-webserver=10.0.2.2 --xre-path=/builds/slave/test/build/hostutils/xre --utility-path=/builds/slave/test/build/hostutils/bin --deviceIP=127.0.0.1 --devicePort=20701 --http-port=8854 --ssl-port=4454 --certificate-path=/builds/slave/test/build/tests/certs --symbols-path=https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/armenzg@mozilla.com-ecdb1c21ae76/try-android-api-9/fennec-37.0a1.en-US.android-arm.crashreporter-symbols.zip --quiet --log-raw=/builds/slave/test/build/blobber_upload_dir/raw_structured_logs.log --total-chunks=2 --test-manifest=gl.json --this-chunk=1
So here, --test-manifest=gl.json is *before* --this-chunk=1 and it passes, so it looks like this problem is fixed. Let me know if this has landed and should I make changes to android_arm_config.py or elsewhere.
Comment 81•10 years ago
|
||
I'm waiting for ahal's review.
I've also re-triggered a couple of jobs that did not come back green.
I will be landing it on Monday.
Comment 82•10 years ago
|
||
I have landed the Mozharness piece.
It will have to be merged to the production branch before it is live.
After that you should be able to work on this. I think.
Comment 83•10 years ago
|
||
Do you know have what you need?
All values in this file are now in use on trunk:
https://hg.mozilla.org/integration/mozilla-inbound/file/03d2076110a3/testing/config/mozharness/android_arm_config.py
Flags: needinfo?(vaibhavmagarwal)
Assignee | ||
Comment 84•10 years ago
|
||
Flags: needinfo?(vaibhavmagarwal)
Reporter | ||
Comment 85•10 years ago
|
||
(In reply to Vaibhav (:vaibhav1994) from comment #84)
> Pushed to try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8b513bbe2f4
Oh no, that didn't run any android tests. You need "-p android-api-9,android-api-11" now.
Assignee | ||
Comment 86•10 years ago
|
||
Oops, didn't know the trychooser syntax changed.
Repushed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=09f592ec13cb
and retriggered the gl jobs as it is not clear whether the test failures are intermittents or not
Reporter | ||
Comment 87•10 years ago
|
||
For both Android 2.3 and Android 4.0, the gl tests' runtestsremote.py command line looks correct, but the jobs are actually running non-gl tests, as though --test-manifest is being ignored. Using 2.3 gl1 as an example:
http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/vaibhavmagarwal@gmail.com-09f592ec13cb/try-android-api-9/try_ubuntu64_vm_mobile_test-mochitest-gl-1-bm51-tests1-linux64-build195.txt.gz
Running on test-1 the command /builds/slave/test/build/venv/bin/python -u /builds/slave/test/build/tests/mochitest/runtestsremote.py --autorun --close-when-done --dm_trans=sut --console-level=INFO --app=org.mozilla.fennec --remote-webserver=10.0.2.2 --xre-path=/builds/slave/test/build/hostutils/xre --utility-path=/builds/slave/test/build/hostutils/bin --deviceIP=127.0.0.1 --devicePort=20701 --http-port=8854 --ssl-port=4454 --certificate-path=/builds/slave/test/build/tests/certs --symbols-path=https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/vaibhavmagarwal@gmail.com-09f592ec13cb/try-android-api-9/fennec-38.0a1.en-US.android-arm.crashreporter-symbols.zip --quiet --log-raw=/builds/slave/test/build/blobber_upload_dir/mochitest-gl-1_raw.log --total-chunks=2 --test-manifest=tests/mochitest/tests/dom/canvas/test/_webgl-conformance.ini --this-chunk=1
...
18:21:19 INFO - Running tests 1-1650/3299
18:21:19 INFO - 0 INFO SimpleTest START
18:21:19 INFO - 1 INFO TEST-START | caps/tests/mochitest/test_app_principal_equality.html
18:21:19 INFO - 2 INFO TEST-OK | caps/tests/mochitest/test_app_principal_equality.html | took 7569ms
18:21:19 INFO - 3 INFO TEST-START | caps/tests/mochitest/test_bug246699.html
18:21:19 INFO - 4 INFO TEST-OK | caps/tests/mochitest/test_bug246699.html | took 4432ms
Comment 88•10 years ago
|
||
that is usually a sign of a manifest path which doesn't exist, then we default to the main one.
how can we confirm that this path exists:
tests/mochitest/tests/dom/canvas/test/_webgl-conformance.ini
I downloaded the tests.zip and it appears to be there and in the right path. Maybe there is some other condition somewhere that is not allowing a custom .ini for remote mochitests?
Assignee | ||
Comment 89•10 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=092179ebafa7
^ Tried with --test-manifest=tests/dom/canvas/test/_webgl-conformance.ini relative to the path where gl.json is present, but it is still failing. Maybe there is some other condition to change..
Comment 90•10 years ago
|
||
oh, I think I see the problem:
for desktop linux we use: --manifest=tests/mochitest/tests/dom/canvas/test/mochitest-subsuite-webgl.ini
for android we use: --test-manifest=tests/dom/canvas/test/_webgl-conformance.ini
a couple things appear to be missing here:
* we should use --manifest, not --test-manifest (https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/mochitest_options.py?from=mochitest_options.py&case=true#300)
* I suspect the path should include tests/mochitest as well
Lets play with tat a bit more and see if we can see green!
Assignee | ||
Comment 91•10 years ago
|
||
Pushed with --manifest and full path: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec27ed298ba1
Reporter | ||
Comment 92•10 years ago
|
||
That didn't work either.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed63119c7707 tries to shed some light on the issue. It introduced some unfortuate failures, but I managed to get some interesting information out of getTestManifest during gl runs.
For Linux:
08:29:47 INFO - getTestManifest -- options.manifestFile=tests/mochitest/tests/dom/canvas/test/mochitest-subsuite-webgl.ini
08:29:47 INFO - getTestManifest -- cwd=/builds/slave/test/build
08:29:47 INFO - getTestManifest -- found file at specified path: /builds/slave/test/build/tests/mochitest/tests/dom/canvas/test/mochitest-subsuite-webgl.ini
For Android:
09:46:35 INFO - getTestManifest -- options.manifestFile=tests/mochitest/tests/dom/canvas/test/_webgl-conformance.ini
09:46:35 INFO - getTestManifest -- cwd=/builds/slave/test/build/tests/mochitest
09:46:35 INFO - getTestManifest -- using masterPath: /builds/slave/test/build/tests/mochitest/tests/mochitest.ini
Note the difference in current working directory.
Comment 93•10 years ago
|
||
aha, so if we set --manifest=tests/dom/canvas/tests/_webgl-conformance.ini that would probably solve the problem!
Assignee | ||
Comment 94•10 years ago
|
||
Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48c03b3f665e
:gbrown, thanks for the insight and helping in this!
Assignee | ||
Comment 95•10 years ago
|
||
Yay! So the Android 2.3 api9 opt gl jobs have passed in the above try push!
Android 4.0 api11+ opt gl job failed, mostly because I did not change it here: https://dxr.mozilla.org/mozilla-central/source/testing/config/mozharness/android_arm_4_4_config.py#30
I will be pushing again by changing that when I get to my desktop.
Reporter | ||
Comment 96•10 years ago
|
||
android_arm_4_4_config.py is for a new type of job that is not running yet -- I wouldn't worry about updating that file.
Android 4.0 api11+ opt jobs normally use android_panda_config.py -- but I don't see mochitest-gl configured in that file. I guess it is using https://dxr.mozilla.org/build:mozharness/source/configs/android/android_panda_releng.py#39?
Comment 97•10 years ago
|
||
My work in bug 1064002 was focused in Android 2.3.
Let me know if I can help you with the 4.0 jobs.
Assignee | ||
Comment 98•10 years ago
|
||
Pushed to try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a91c9094fe8
This patch points to my mozharness repo: http://hg.mozilla.org/users/vaibhavmagarwal_gmail.com/vaibhavmagarwal_repo/ so hopefully, the 4.0 jobs should run as expected.
Assignee | ||
Comment 99•10 years ago
|
||
The android 4.0 gl job is still failing in https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a91c9094fe8
I have made changes to android_panda_config.py and androidx86.py in my repo: http://hg.mozilla.org/users/vaibhavmagarwal_gmail.com/vaibhavmagarwal_repo/rev/94cad7bad7f4 , but it looks like either that is not affecting the changes or it is not picking up the changes. The script_revlink is showing this: http://hg.mozilla.org/users/vaibhavmagarwal_gmail.com/vaibhavmagarwal_repo//rev/7ebb4b339e48 but it should be http://hg.mozilla.org/users/vaibhavmagarwal_gmail.com/vaibhavmagarwal_repo//rev/94cad7bad7f4 . Does anyone knows why this is happening?
Assignee | ||
Comment 100•10 years ago
|
||
Retriggered the gl jobs after updating the tip of my repo to production: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a91c9094fe8
Assignee | ||
Comment 101•10 years ago
|
||
So the android 4.0 gl job is green on try! Armen, please review this patch :)
Attachment #8553268 -
Flags: review?(armenzg)
Assignee | ||
Comment 102•10 years ago
|
||
:gbrown, please review this patch, it passes on try
Attachment #8553273 -
Flags: review?(gbrown)
Comment 103•10 years ago
|
||
Comment on attachment 8553268 [details] [diff] [review]
mozharness.patch
Unfortunately, this works on try but it does not mean that it won't burn jobs in some other release tree.
Let's fix this in-tree.
Please add a mochitest-gl category [1][2]:
* by copying the mochitest section
* extend both by adding "--test-manifest=gl.json"
This is a no-op change. Push it to try to make sure nothing is affected.
Once that change lands, we need to check that the mochitest-gl commands on aurora, beta, release and esr31 are the same. This can be determined by looking at the logs of the jobs for Android 4.0 and Android x86 jobs.
If it is all the same, we can then start uplifting everywhere. I can help.
Once all release branches are working properly. We need to do the following:
* Change the two files modified in this patch to do the following:
** "mochitest-gl": [] <-- We stop appending -manifest
** We change the category of "mochitest-gl" jobs to be "mochitest-gl"
*** This will make it start using the category from the in-tree configs
Push to try and make sure it works. You will need to pin to your user repo.
If it works, we can then land it and be done.
The other option is to wait for bug 1110286 to be done (next few weeks) and then this patch would actually not be a problem to land.
[1] http://hg.mozilla.org/mozilla-central/file/default/testing/config/mozharness/android_panda_config.py#l70
[2] http://hg.mozilla.org/mozilla-central/file/default/testing/config/mozharness/android_x86_config.py#l7
Attachment #8553268 -
Flags: review?(armenzg) → review-
Reporter | ||
Comment 104•10 years ago
|
||
Comment on attachment 8553273 [details] [diff] [review]
gl.patch
Review of attachment 8553273 [details] [diff] [review]:
-----------------------------------------------------------------
This looks fine, but let's follow through on Armen's advice (comment 103) before considering landing.
Attachment #8553273 -
Flags: review?(gbrown)
Assignee | ||
Comment 105•10 years ago
|
||
Let's do this! Pushed to try by adding mochitest-gl to in-tree configs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9081ed351447
Assignee | ||
Comment 106•10 years ago
|
||
:armenzg, the push on try is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9081ed351447
so does it mean, we can land https://hg.mozilla.org/try/rev/e10d62259032 ?
Flags: needinfo?(armenzg)
Comment 107•10 years ago
|
||
Yes, once we review it, however, it is not being used.
It only gets used if on mozharness configs we change the category to be "mochitest-gl" instead of "mochitest.
Flags: needinfo?(armenzg)
Comment 108•10 years ago
|
||
<vaibhav1994> armenzg_mtg: if I just remove this line: https://dxr.mozilla.org/build:mozharness/source/configs/android/android_panda_releng.py#39, will it not make it use my in-tree configs?
If you remove it, you will get every "mochitest" parameter plus nothing.
The problem is that we need to read from the new "mochitest-gl" section, however, we don't have that ability for android_panda.py.
Let me describe why they're a bit different.
You can change the category for android 2.3 and android x86 jobs because they both use a new format for the mozharness configs [1], which reads first from the key "test_suite_definitions" and then from the in-gecko configs (from the category you specify)
> "test_suite_definitions": {
> "jsreftest": {
> "category": "reftest",
> "extra_args": ["../jsreftest/tests/jstests.list",
> "--extra-profile-file=jsreftest/tests/user.js"]
> },
Unfortunately, Android 2.3 uses the script android_panda_releng.py [2]
Which chooses the category like this:
> if '%s_options' % suite_category in self.tree_config:
instead of like this:
> suite_category = self.test_suite_definitions[suite_name]["category"]
In your case, suite_category is "mochitest" rather than "mochitest-gl".
You could work on changing android_panda_releng.py to read the new format.
[1] http://hg.mozilla.org/build/mozharness/file/default/scripts/android_emulator_unittest.py#l392
[2] http://hg.mozilla.org/build/mozharness/file/9892549f8789/scripts/android_panda.py#l465
Assignee | ||
Comment 109•10 years ago
|
||
Armen, thanks for taking the time and explaining in full detail, I now understand the problem fully.
I understand that android_panda_releng.py needs to be converted to the new format, but it involves converting not only mochitests but also reftests, crashtest, jsreftest, robocop, xpcshell, jittest, cppunittests for android jobs. Plus one will also need to change the logic in android_panda.py for the new format. This sounds like a project in itself :)
I can place a *hack* in android_panda.py (somewhere here- [1]) to read from "mochitest_gl" instead of "mochitest" till the time the full change for new format takes place. Let me know if that works for you in this case. Else, if there is already work being done for converting android_panda_releng.py to new format, I can maybe in that bug and when that gets complete, then we can complete this bug.
[1] http://hg.mozilla.org/build/mozharness/file/9892549f8789/scripts/android_panda.py#l246
Flags: needinfo?(armenzg)
Comment 110•10 years ago
|
||
It is a resourceful hack you propose, however, you will have to make sure that the change will allow for riding the trains. In other words, you might use the hack while still running a specific version of Gecko.
I highly suggest waiting for bug 1110286 to be done and this way we won't have to worry about backwards compatibility.
Flags: needinfo?(armenzg)
Assignee | ||
Comment 111•10 years ago
|
||
After chatting with Armen on IRC, I am trying out my hack to see if it works.
Pushed to treeherder, where it points to my mozharness repo and should use in-tree configs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e30ff3fddf07
Assignee | ||
Comment 112•10 years ago
|
||
After some debugging and retrying jobs, Android 4.0 API11+ 'gl' job is now working for in-tree configs for try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e30ff3fddf07 . The corresponding patch for my mozharness user repo is - http://hg.mozilla.org/users/vaibhavmagarwal_gmail.com/vaibhavmagarwal_repo/rev/f7d6ffc2abb0
Armen, could you please see the patch of my mozharness repo and let me know if we can move forward with this hack? This hack will move mochitest-gl to in-tree configs and then we can remove gl.json.
Flags: needinfo?(armenzg)
Comment 113•10 years ago
|
||
Your mh changes seem correct.
Can you please test these mozharness changes based on mc? All three android platforms. Even if some jobs fail (see advice below).
You also need a separate push with any needed in-tree changes. For instance androidx86's mochitest-gl missing category.
Could you also please attach a patch from your mozharness showing any differences from the official mozharness repo?
Side advise, try not to push to try from your own heads. Always rebase from mc or mi.
The parent of e30ff3fddf07 is e10d62259032 which hides code that you're trying. In this case:
https://hg.mozilla.org/try/diff/e10d62259032/testing/config/mozharness/android_panda_config.py
Pushing from your own heads makes it hard for me (and others) to see which other changes are being tested.
[1] http://hg.mozilla.org/mozilla-central/file/default/testing/config/mozharness/android_x86_config.py
Flags: needinfo?(armenzg)
Assignee | ||
Comment 114•10 years ago
|
||
try push based on m-i: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ec0d1c75779
Assignee | ||
Comment 115•10 years ago
|
||
Carrying forward the r+ by :armenzg in bug 1131184. Not to land till bug 1110286 is fixed.
Attachment #8561019 -
Attachment is obsolete: true
Attachment #8561529 -
Flags: review+
Assignee | ||
Comment 116•10 years ago
|
||
Moving mochitest-gl to in-tree configs. :gbrown, please review this. It is safe to check this in as it is not used currently - will be when we check-in the mozharness changes. related try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ec0d1c75779
Attachment #8561545 -
Flags: review?(gbrown)
Reporter | ||
Updated•10 years ago
|
Attachment #8561545 -
Flags: review?(gbrown) → review+
Assignee | ||
Comment 117•10 years ago
|
||
Hey Armen, I see bug 1110286 is fixed. Now can I go ahead with the mozharness fix and move mochitest-gl to in-tree?
Flags: needinfo?(armenzg)
Comment 118•10 years ago
|
||
Yes, this is correct.
I completely forgot about this. My apologies.
For a mozharness repo change to be effective will require bumping testing/mozharness/mozharness.json.
Flags: needinfo?(armenzg)
Updated•10 years ago
|
Whiteboard: [leave open] → [leave open][ateam_harness_work]
Assignee | ||
Comment 119•10 years ago
|
||
try push to see if the changes are still valid: https://treeherder.mozilla.org/#/jobs?repo=try&revision=43365778ef8d
Assignee | ||
Comment 120•10 years ago
|
||
Armen, the 'gl' jobs for android on the above try push are passing (there are other jobs left to be done just for a check, but the ones which we care about are passing). Please review this patch and since bug 1110286 is fixed, can we land this?
Attachment #8553268 -
Attachment is obsolete: true
Attachment #8576215 -
Flags: review?(armenzg)
Assignee | ||
Comment 121•10 years ago
|
||
:gbrown could you please review this. Thanks
Attachment #8553273 -
Attachment is obsolete: true
Attachment #8576216 -
Flags: review?(gbrown)
Reporter | ||
Comment 122•10 years ago
|
||
Comment on attachment 8576216 [details] [diff] [review]
gl.patch
Review of attachment 8576216 [details] [diff] [review]:
-----------------------------------------------------------------
Please update testing/config/mozharness/android_arm_4_3_config.py as well. It is not being used in any production jobs currently, but will be in use soon.
Attachment #8576216 -
Flags: review?(gbrown) → review+
Assignee | ||
Comment 123•10 years ago
|
||
Updated the patch. Carrying forward the r+
Attachment #8576216 -
Attachment is obsolete: true
Attachment #8576636 -
Flags: review+
Comment 124•10 years ago
|
||
Comment on attachment 8576215 [details] [diff] [review]
mozharness.patch
Review of attachment 8576215 [details] [diff] [review]:
-----------------------------------------------------------------
Remember that once landed on mozharnes, grab the revision and we need to land a mozharness.json file on the tree with that revision.
Attachment #8576215 -
Flags: review?(armenzg) → review+
Assignee | ||
Comment 125•10 years ago
|
||
Please checkin the mozharness.patch only for now.
Keywords: checkin-needed
Comment 126•10 years ago
|
||
landed the mozharness patch that armen reviewed:
https://hg.mozilla.org/build/mozharness/rev/9e077d6e8a7f
Assignee | ||
Comment 127•10 years ago
|
||
Armen, :gbrown has reviewed this patch once. I just wanted you to see the mozharness.json file, I have changed the revision to "9e077d6e8a7f". If it is correct, I will check this patch in too. Thanks
Attachment #8576636 -
Attachment is obsolete: true
Attachment #8576651 -
Flags: review?(armenzg)
Comment 128•10 years ago
|
||
Comment on attachment 8576651 [details] [diff] [review]
gl.patch
That's correct.
Once you landed, only that changeset and forward will start using the new mozharness revision.
Sheriffs should be able to back you out if it was needed.
The mozharness change would also need to be backed out if the sheriffs backed out the m-i change.
I don't think a back out would be needed but wanted to give you all the info since this is a new process for most people.
Attachment #8576651 -
Flags: review?(armenzg) → review+
Assignee | ||
Comment 129•10 years ago
|
||
Please check-in gl.patch. Thanks!
Comment 130•10 years ago
|
||
Comment 131•10 years ago
|
||
the commit message had the wrong bug, it has bug 1083346 instead :(
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 132•10 years ago
|
||
Pushed to try for removing android.json, android23.json and androidx86.json: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce5205793d86
Comment 133•10 years ago
|
||
mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/d35bfd399151
Comment 134•10 years ago
|
||
Assignee | ||
Comment 135•10 years ago
|
||
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce5205793d86 is green on android jobs till now. I have re-triggered some more. In the meantime, I would like :gbrown and :jmaher to review this mozharness patch so that we can land this *if* the remaining jobs triggers are up to expectation.
Attachment #8561529 -
Attachment is obsolete: true
Attachment #8561545 -
Attachment is obsolete: true
Attachment #8577315 -
Flags: review?(jmaher)
Attachment #8577315 -
Flags: review?(gbrown)
Comment 136•10 years ago
|
||
Comment on attachment 8577315 [details] [diff] [review]
remove_androidjson_mozharness.patch
Review of attachment 8577315 [details] [diff] [review]:
-----------------------------------------------------------------
looks good to me. I assume once we land this on mozharness, then we need to get it in production and finally update mozilla-inbound.
Attachment #8577315 -
Flags: review?(jmaher) → review+
Reporter | ||
Updated•10 years ago
|
Attachment #8577315 -
Flags: review?(gbrown) → review+
Comment 137•10 years ago
|
||
mozharness patch landed:
https://hg.mozilla.org/build/mozharness/rev/1deaa6e0dc43
Assignee | ||
Comment 138•10 years ago
|
||
:jmaher please review this and check it in!
Attachment #8577388 -
Flags: review?(jmaher)
Comment 139•10 years ago
|
||
Comment on attachment 8577388 [details] [diff] [review]
android_final.patch
Review of attachment 8577388 [details] [diff] [review]:
-----------------------------------------------------------------
thanks, this is nice to see. I would like a followup patch to remove the code in runtests.py/etc. for handling .json manifests.
Attachment #8577388 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 140•10 years ago
|
||
Yes, I will make another try push for it. Also I think we need to change buildbot configs to remove references to android*.json after this patch gets merged in m-c.
Comment 141•10 years ago
|
||
Updated•10 years ago
|
Whiteboard: [leave open][ateam_harness_work] → [ateam_harness_work]
Comment 142•10 years ago
|
||
we need to land lots of this on aurora/beta. Next week we can build up the patches to uplift as well as figure out what rev of mozharness to uplift- oh joy!
Depends on: 1143227
Comment 143•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Reporter | ||
Comment 144•10 years ago
|
||
It looks like this has started to cause problems on aurora:
http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-aurora-android-api-11/1426287315/mozilla-aurora_panda_android_test-mochitest-gl-bm102-tests1-panda-build69.txt.gz
17:09:48 INFO - Copy/paste: python -u /builds/panda-0527/test/build/tests/mochitest/runtestsremote.py --deviceIP=10.26.133.70 --xre-path=../hostutils/xre --utility-path=../hostutils/bin --certificate-path=certs --app=org.mozilla.fennec_aurora --console-level=INFO --http-port=30527 --ssl-port=31527 --run-only-tests=android.json --symbols-path=https://ftp-ssl.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-aurora-android-api-11/1426287315/fennec-38.0a2.en-US.android-arm.crashreporter-symbols.zip --quiet --log-raw=/builds/panda-0527/test/build/blobber_upload_dir/mochitest-gl_raw.log --manifest tests/dom/canvas/test/_webgl-conformance.ini
...
17:09:49 INFO - Usage: runtestsremote.py [options]
17:09:49 INFO - runtestsremote.py: error: Unable to support both --manifest and --test-manifest/--run-only-tests at the same time
17:09:49 ERROR - Return code: 2
17:09:49 ERROR - No tests run or test summary not found
Comment 145•10 years ago
|
||
aurora got mozharness updates which rely on test harness updates as well. We still need to fix webgl in general in bug 1143218 (this week), so the road to completion has more work.
Tomorrow I could help uplift stuff if needed.
Comment 146•10 years ago
|
||
It seems that the Aurora issue was fixed with this:
https://hg.mozilla.org/releases/mozilla-aurora/rev/bdfe1112a65c
You need to log in
before you can comment on or make changes to this bug.
Description
•