668676 - mozprofile magic marker search is broken

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Jeff Hammel]

https://github.com/mozautomation/mozmill/blob/master/mozprofile/mozprofile/profile.py#L123

we use `list.index` to find the beginning and terminating section of
"#MozRunner Prefs {Start,End}" delimeters and remove these lines.

However,
https://github.com/mozautomation/mozmill/blob/master/mozprofile/mozprofile/profile.py#L102
is a "public" function and can be called more than once....and is!

https://github.com/mozautomation/mozmill/blob/master/mozrunner/mozrunner/runner.py#L288

in addition to the call from `Profile.__init__`.  

So we are cleaning up the first case but no subsequent cases :(

There are a few solutions to this problem:

1. (Recommended, *and* the easiest) Check for all of these
delimeters.  Ensure they're in a sane order and non-overlapping
regions

2. make Profile.set_preferences not callable.  This isn't really
possible with how mozrunner deals with profiles. So no-go here.

3. Rework how we deal with and read preferences, a la
https://bugzilla.mozilla.org/show_bug.cgi?id=661026 . However, after
some thinking about it, I'm not sure if this is a good idea either.
If you have a pref in a profile that is set by the user (or at least
not by mozprofile), then you have to deal with what preferences you
set, what preferences the user is set internal to mozmill.  As much as
I dislike the delimeters, maybe they do make sense after all ;)  I'm
not going to close bug 661206 yet, though, as it does make sense for
e.g. combining several prefs.js files for e.g. a mozmill run

So I'm thinking 1. :)  I may fix this as part of bug 668249 since
that's where I found it and I'm already in the neighborhood

Comment 1

[cmtalbert]

(In reply to comment #0)
> So I'm thinking 1. :)  I may fix this as part of bug 668249 since
> that's where I found it and I'm already in the neighborhood
Sounds good to me.

Comment 2

[Jeff Hammel]

So https://bug668249.bugzilla.mozilla.org/attachment.cgi?id=543492 fixes this issue and introduces a test, so if that lands we can close this too

Comment 3

[Jeff Hammel]

fixed with https://github.com/mozautomation/mozmill/commit/aa3afa9a5543cd69292b0e4a731dcd88e49af030 landed for bug 668249

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This has already been fixed for Mozmill 2.