896269 - new mozprofile enhancements: profile-view, profile-diff

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Jeff Hammel]

At several points in time -- as you might expect, when debugging
profiles often but not only when changing profile test code -- I have
wanted a succinct but sufficient view of what was in a profile so that
I could track down the issue more easily.  (E.g. if something works
fine locally, but fails on a test machine, then turnaround and
debugging ain't fast; but even locally, I have often wished for such
facilities.)

Likewise, and especially when comparing runs where profile code is
changed, I haven't wanted to "diff" two profiles.

For both cases, I have ended up mostly hacking what I needed into the
harness when I became that desparate.  The results were non portable
and usually were a last resort thing, because tooling the tools
(especially non-portably) to this level is usually something I do only
when perceived (probably wrongly, if here we are) quicker methods fail
to yield results.

Proposed at *very* low priority: two new console_scripts for
mozprofile: profile-view and profile-diff.

profile-view gives what is in the profile.  A first round could just
be a file tree (maybe including sizes) and prefs.  But you can imagine
this growing on an as-need basis.  What about extensions? etc etc etc

profile-diff gives a diff of the profiles, again to be written
incrementally.

Pretty open on how to begin.  Of course it'd be nice to dump this out
from the Profile class as well as from the command line;  but having
both is easy and doesn't hurt anyone.

I'm happy to give input here, but I doubt I'll be programming this any
time soon.....*UNTIL* the next time I needed it yesterday!

Comment 1

[Jeff Hammel]

More out of necessity than any other reason, I have put up some of a profile summary function as https://bugzilla.mozilla.org/attachment.cgi?id=780808 as part of bug 688667 ; a diff function should be pretty straightforward from here

Comment 2

[Jeff Hammel]

Attachment: port of original attachment

Comment 3

[Jeff Hammel]

Attachment: bug-896269.diff

This is a pretty rough 1st draft.  I vote for landing as A. it shouldn't break anything; B. we can refine it later; and C. I don't really have time to iterate much on it.  I'm happy to take away stuff, but iterative refinement is....time consuming.

This will require a version bump to mozfile and mozprofile

Comment 4

[Jonathan Griffin (:jgriffin)]

Comment on attachment 782861 [details] [diff] [review]
bug-896269.diff

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

Cool.  Definitely incomplete, but definitely worth landing as-is and iterating on later when needed.

::: mozprofile/mozprofile/profile.py
@@ +258,5 @@
>      __del__ = cleanup
>  
> +    ### methods for introspection
> +
> +    def print_profile(self, return_parts=False):

Should this method be named differently?  It doesn't actually print anything.  Calling:

print profile.print_profile()

looks redundant to me.  Maybe to_string()?  I'm definitely not very good at naming either.

@@ +278,5 @@
> +                prefs = Preferences.read_prefs(path)
> +                if prefs:
> +                    prefs = dict(prefs)
> +                    parts.append((prefs_file,
> +                    '\n%s' %('\n'.join(['%s: %s' % (key, ('%s...' % prefs[key][:75]) if key == 'network.proxy.autoconfig_url' else prefs[key])

Should we document where this magic '75' comes from?

Comment 5

[Jeff Hammel]

(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> Comment on attachment 782861 [details] [diff] [review]
> bug-896269.diff
> 
> Review of attachment 782861 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Cool.  Definitely incomplete, but definitely worth landing as-is and
> iterating on later when needed.
> 
> ::: mozprofile/mozprofile/profile.py
> @@ +258,5 @@
> >      __del__ = cleanup
> >  
> > +    ### methods for introspection
> > +
> > +    def print_profile(self, return_parts=False):
> 
> Should this method be named differently?  It doesn't actually print
> anything.  Calling:
> 
> print profile.print_profile()
> 
> looks redundant to me.  Maybe to_string()?  I'm definitely not very good at
> naming either.

Maybe summary()?

> 
> @@ +278,5 @@
> > +                prefs = Preferences.read_prefs(path)
> > +                if prefs:
> > +                    prefs = dict(prefs)
> > +                    parts.append((prefs_file,
> > +                    '\n%s' %('\n'.join(['%s: %s' % (key, ('%s...' % prefs[key][:75]) if key == 'network.proxy.autoconfig_url' else prefs[key])
> 
> Should we document where this magic '75' comes from?

Maybe ;) its really "not to fill up the whole screen with crap"; it should be more like 80 - len('network.proxy.autoconfig_url') - len(': ') - buffer, where buffer is like, I dunno, 10 or so.  I also currently only do this for this preference; of course, a more general treatment would be nice.  Will note in comment

Comment 6

[Jonathan Griffin (:jgriffin)]

summary() sounds good to me!

Comment 7

[Jeff Hammel]

pushed: https://github.com/mozilla/mozbase/commit/c9ea6860eea0bc38741f6665cfe1a488f788293c