Closed
Bug 1180275
Opened 9 years ago
Closed 9 years ago
Fix the condition to disable test_app_protocol.html
Categories
(Core :: DOM: Service Workers, defect)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(2 files)
1.73 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
862 bytes,
patch
|
ferjm
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8629473 -
Flags: review?(ferjmoreno)
Assignee | ||
Updated•9 years ago
|
Attachment #8629472 -
Flags: review?(ted)
Comment 3•9 years ago
|
||
Comment on attachment 8629473 [details] [diff] [review] Part 2: Disable test_app_protocol.html in release builds Review of attachment 8629473 [details] [diff] [review]: ----------------------------------------------------------------- Thank you Ehsan! We'll need to update https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/build/buildsystem/mozinfo.html#mozinfo-attributes as well.
Attachment #8629473 -
Flags: review?(ferjmoreno) → review+
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #3) > We'll need to update > https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/build/ > buildsystem/mozinfo.html#mozinfo-attributes as well. Yep, that's what the mozinfo.rst hunk in my other patch does. :-)
Updated•9 years ago
|
Attachment #8629472 -
Flags: review?(ted) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/56e9597b1257 https://hg.mozilla.org/integration/mozilla-inbound/rev/9a048b598b1e
Comment 6•9 years ago
|
||
Backed out for OSX test_android_eclipse.py failures that didn't go away with a clobber either. https://treeherder.mozilla.org/logviewer.html#?job_id=11745071&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/41fd23011c72
Assignee | ||
Comment 7•9 years ago
|
||
This is really weird. See the log message: 19:02:07 WARNING - TEST-UNEXPECTED-FAIL | /builds/slave/m-in-m64-000000000000000000000/build/src/python/mozbuild/mozbuild/test/backend/test_android_eclipse.py | line 89, test_referenced_projects: 'bool' object has no attribute '__getitem__' 19:02:07 INFO - ERROR: Ensure we add class path entries to extra jars iff asked to. 19:02:07 INFO - Traceback (most recent call last): 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/python/mozbuild/mozbuild/test/backend/test_android_eclipse.py", line 99, in test_extra_jars 19:02:07 INFO - self.env = self._consume('android_eclipse', AndroidEclipseBackend) 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/python/mozbuild/mozbuild/test/backend/common.py", line 119, in _consume 19:02:07 INFO - env, objs = self._emit(name, env=env) 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/python/mozbuild/mozbuild/test/backend/common.py", line 114, in _emit 19:02:07 INFO - emitter = TreeMetadataEmitter(env) 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 102, in __init__ 19:02:07 INFO - mozinfo.find_and_update_from_json(config.topobjdir) 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/testing/mozbase/mozinfo/mozinfo/mozinfo.py", line 169, in find_and_update_from_json 19:02:07 INFO - update(json_path) 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/testing/mozbase/mozinfo/mozinfo/mozinfo.py", line 142, in update 19:02:07 INFO - sanitize(info) 19:02:07 INFO - File "/builds/slave/m-in-m64-000000000000000000000/build/src/testing/mozbase/mozinfo/mozinfo/mozinfo.py", line 117, in sanitize 19:02:07 INFO - if release[:4] >= "10.6": # Note this is a string comparison 19:02:07 INFO - TypeError: 'bool' object has no attribute '__getitem__' 19:02:07 INFO - ERROR: Ensure we include another project correctly. 19:02:07 INFO - Traceback (most recent call last): This is the code trying to access the release variable: <http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozinfo/mozinfo/mozinfo.py#117> But this variable should be defined here: <http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozinfo/mozinfo/mozinfo.py#36> but it seems like on line 117, |release| is referring to info['release'] for some reason, which would be a boolean after my patch! And I can't reproduce locally when I run make check. Can someone with a stronger python-fu explain to me why release there is being bound to a property of the info object?
Flags: needinfo?(ted)
Flags: needinfo?(ahalberstadt)
Comment 8•9 years ago
|
||
I didn't spec out the entire order or execution, but it likely has something to do with this: https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozinfo/mozinfo/mozinfo.py#143 That is a kind of terrible hack that let's people use shortcuts like 'mozinfo.isWin'. Instead of globals(), we should probably define __all__ to explicitly list which parameters we want people to have access too (though doing this won't fix your problem if we need to expose mozinfo.release anyway).
Flags: needinfo?(ted)
Flags: needinfo?(ahalberstadt)
Comment 9•9 years ago
|
||
Yeah, mozinfo has a global named release and when you put something named that in mozinfo.json mozinfo stomps on its own variable. Pretty dumb. The simple workaround would be to just rename this to "release_build" or something. We could also fix mozinfo, but I don't know how involved that'd be.
Assignee | ||
Comment 10•9 years ago
|
||
OK, I'll rename my variable! Thanks for teaching my (why I should dislike) Python :-)
Comment 11•9 years ago
|
||
Not really python's fault, it's a pretty gross abuse on our part :)
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fcf8a3fee0a https://hg.mozilla.org/integration/mozilla-inbound/rev/5da3d259997e
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #11) > Not really python's fault, it's a pretty gross abuse on our part :) Fair!
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2fcf8a3fee0a https://hg.mozilla.org/mozilla-central/rev/5da3d259997e
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•