pageloader needs to log to stdout and have the ability to install in a profile directory as an extension

RESOLVED FIXED

Status

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mobile_unittests])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
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

9 years ago
Attachment #478398 - Flags: review?(vladimir)
(Assignee)

Updated

9 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

9 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

9 years ago
Attachment #478599 - Flags: review?(vladimir) → review+

Comment 3

9 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

9 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

9 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?
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.
Also - the bundle is provided as part of standalone talos - are there any other pageloader tests that are run without talos?
(Assignee)

Comment 11

9 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

9 years ago
There has been no activity on this bug since last Thursday, can we resolve this today?
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
(Assignee)

Comment 14

9 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

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Blocks: 602055
You need to log in before you can comment on or make changes to this bug.