917576 - Regression: Ru tests not in fact running un-accelerated

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Nick Cameron [:nrc]]

Because reftest is not parsing non-string prefs.

Comment 1

[Nick Cameron [:nrc]]

Attachment: parse non-string pref values

Comment 2

[Nick Cameron [:nrc]]

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.

Comment 3

[Nick Cameron [:nrc]]

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

Comment 4

[Nick Cameron [:nrc]]

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

Could be interesting...

Comment 5

[Nick Cameron [:nrc]]

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

Comment 6

[Nick Cameron [:nrc]]

Bug 914521 causes bustage on Cipc.

Comment 7

[Nick Cameron [:nrc]]

Attachment: disable the failing cipc test

Comment 8

[Andrew Halberstadt [:ahal]]

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

Comment 9

[Andrew Halberstadt [:ahal]]

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

Comment 10

[Ed Morley [:emorley]]

This was regressed by bug 899171 which is in mozilla26.

Comment 11

[Nick Cameron [:nrc]]

Attachment: parse non-string pref values using cast

much easier :-)

Comment 12

[Andrew Halberstadt [:ahal]]

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]]'

Comment 13

[Nick Cameron [:nrc]]

Attachment: parse non-string pref values using cast

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

Comment 14

[Andrew Halberstadt [:ahal]]

Comment on attachment 806825 [details] [diff] [review]
parse non-string pref values using cast

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

Thanks!

Comment 15

[Nick Cameron [:nrc]]

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

Comment 16

[Nick Cameron [:nrc]]

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

Comment 17

[Ed Morley [:emorley]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(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