Because reftest is not parsing non-string prefs.

We need to check this kind of thing in the automation too, so we notice these bugs in future. I think it took four weeks for anyone to notice this.

I bet we have not been running multi-process tests as multi-process either, for the same reason.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=7e1feb223053 Could be interesting...

I think bug 899171 regressed this, but I have not confirmed.

Bug 914521 causes bustage on Cipc.

Comment on attachment 806302 [details] [diff] [review] parse non-string pref values Review of attachment 806302 [details] [diff] [review]: ----------------------------------------------------------------- Sorry about the regression, and thanks for catching this! There's a method in mozprofile that already has this logic though. ::: layout/tools/reftest/runreftest.py @@ +85,5 @@ > + else: > + try: > + prefs[thispref[0]] = int(prefValue) > + except ValueError: > + prefs[thispref[0]] = prefValue This whole chunk can just be: prefValue = mozprofile.Preferences.cast(thispref[1].strip()) http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozprofile/mozprofile/prefs.py#57

Er, I meant 'prefs[thispref[0]] = ...'

This was regressed by bug 899171 which is in mozilla26.

much easier :-)

Comment on attachment 806822 [details] [diff] [review] parse non-string pref values using cast Review of attachment 806822 [details] [diff] [review]: ----------------------------------------------------------------- You forgot to do 'prefs[thispref[0]]' but consider this an r+ with that fixed :) ::: layout/tools/reftest/runreftest.py @@ +78,5 @@ > thispref = v.split('=') > if len(thispref) < 2: > print "Error: syntax error in --setpref=" + v > sys.exit(1) > + thispref[0] = mozprofile.Preferences.cast(thispref[1].strip()) This needs to be 'prefs[thispref[0]]'

whoops, that what I get for writing patches before my morning coffee

Comment on attachment 806825 [details] [diff] [review] parse non-string pref values using cast Review of attachment 806825 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!

Comment on attachment 806825 [details] [diff] [review] parse non-string pref values using cast [Approval Request Comment] Bug caused by (feature/regressing bug #): 899171 User impact if declined: No functioning Ru/Cipc/Ripc tests Testing completed (on m-c, etc.): Try (only affects tests) Risk to taking this patch (and alternatives if risky): Low (no alternatives) String or IDL/UUID changes made by this patch: none

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f8518a7584d https://hg.mozilla.org/integration/mozilla-inbound/rev/4a5e5dfa5e2f

https://hg.mozilla.org/mozilla-central/rev/5f8518a7584d https://hg.mozilla.org/mozilla-central/rev/4a5e5dfa5e2f

Comment on attachment 806825 [details] [diff] [review] parse non-string pref values using cast This is NPOTB. I can land without approval. I will get it in my next round of uplifts to Aurora.

(In reply to Nick Cameron [:nrc] from comment #2) > We need to check this kind of thing in the automation too, so we notice > these bugs in future. I think it took four weeks for anyone to notice this. Agreed! But I don't see that this ever happened? https://hg.mozilla.org/releases/mozilla-aurora/rev/ded6ef58e8a3 https://hg.mozilla.org/releases/mozilla-aurora/rev/c3cd681e553d