Closed
Bug 599494
Opened 14 years ago
Closed 14 years ago
pageloader needs to log to stdout and have the ability to install in a profile directory as an extension
Categories
(Testing :: Talos, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
(Whiteboard: [mobile_unittests])
Attachments
(1 file, 2 obsolete files)
8.13 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
in order to work well in android automation, we need to install extensions in the profile and write all output to a direct file, not stdout.
Assignee | ||
Updated•14 years ago
|
Attachment #478398 -
Flags: review?(vladimir)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [mobile_unittests]
Comment on attachment 478398 [details] [diff] [review] log to stdout and bundle as an extension (1.0) the install.rdf file is in the wrong place, no? other changes look fine, but who calls dumpLog?
Assignee | ||
Comment 2•14 years ago
|
||
slight update to cleanup the mozillafilelogger.js code. To answer the questions in the previous comment: install.rdf is in the root directory (where chrome.manifest is located at.) This works for installing in a profile. Please advise if you think we should put install.rdf in a different location. for the dumpLog() call, it isn't really used. pageloader already uses dumpLine() which is pretty much what dumpLog() did. I removed dumpLog() since it wasn't used and kept my MozillaFileLogger.log() call inside of dumpLine().
Assignee: nobody → jmaher
Attachment #478398 -
Attachment is obsolete: true
Attachment #478599 -
Flags: review?(vladimir)
Attachment #478398 -
Flags: review?(vladimir)
Updated•14 years ago
|
Attachment #478599 -
Flags: review?(vladimir) → review+
Comment 3•14 years ago
|
||
Comment on attachment 478599 [details] [diff] [review] log to stdout and bundle as an extension (1.1) looks ok to me, vlad's review still welcome, obviously
Attachment #478599 -
Flags: review?(vladimir)
Comment on attachment 478599 [details] [diff] [review] log to stdout and bundle as an extension (1.1) won't the call in dumpLine error out if MozillaFileLogger == null? Or if that's fine, then why is there a check for it being non-null before calling close? Also, I guess I don't understand where the root is of what this is patching; I assumed layout/tools/pageloader, but that doesn't seem to be the case?
Assignee | ||
Comment 5•14 years ago
|
||
I updated the patch to include a check for MozillaFileLogger on the log() call. Thanks for the feedback on that. As for the root of this, here is where it will live: http://hg.mozilla.org/build/pageloader Essentially this will be cloned into a profile directory similar to this: cd profile/extensions mkdir pageloader@mozilla.org hg clone http://hg.mozilla.org/build/pageloader pageloader@mozilla.org If you would like me to change things to work with other locations of pageloader, let me know and I can figure it out.
Attachment #478599 -
Attachment is obsolete: true
Attachment #479627 -
Flags: review?(vladimir)
Attachment #478599 -
Flags: review?(vladimir)
Hmmm.. so the problem is that I believe the pageloader that's checked into the tree is included with --enable-tests in the dist/bin dir. So then we'll have an extension providing the same thing. Could lead to problems, but even more of an issue, I don't like that we have a fork. Cc'ing dbaron, who owns the in-tree layout tools stuff (I believe?). Is moving the code to build/pageloader for simplicity for releng? Any chance you guys can just pull from the source tree? I'm hesitant to suggest removing the in-tree version, since it's useful for people doing local testing without needing to pull a second repo...
I really don't know anything about the in-tree pageloader.
Assignee | ||
Comment 8•14 years ago
|
||
alice, can you comment on the hg.mozilla.org/build/pageloader vs mozilla-central in tree version? I suspect it has to do with supporting all branches?
Comment 9•14 years ago
|
||
The version in build/pageloader is a bundle for bug 580698. We currently install the bundle version for all talos tests - the version in the mozilla source tree is essentially ignored.
Comment 10•14 years ago
|
||
Also - the bundle is provided as part of standalone talos - are there any other pageloader tests that are run without talos?
Assignee | ||
Comment 11•14 years ago
|
||
:vlad, are there any other concerns you have with landing this patch on the hg.m.o/build/pageloader repository?
Assignee | ||
Comment 12•14 years ago
|
||
There has been no activity on this bug since last Thursday, can we resolve this today?
Comment 13•14 years ago
|
||
Is there a chance we can get this reviewed or acted on - I would like to wrap up our initial list of tests and this one is blocking
Attachment #479627 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 14•14 years ago
|
||
landed in hg repo: http://hg.mozilla.org/build/pageloader/rev/59d4f04497dd we still need to add this to the list of releng downtime tasks.
Assignee | ||
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•