977833 - gfxPrefs not initialized for GTest runs

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

Initialize gfxPrefs for the test runs to avoid the assert of initializing it at the first preference access.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Initialize gfxPrefs in GTestRunner

Or, should we initialize this in each individual test? With bug 971943, ASync... test needs it.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

https://tbpl.mozilla.org/?tree=Try&rev=e72ad8378060

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 8383295 [details] [diff] [review]
Initialize gfxPrefs in GTestRunner

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

::: testing/gtest/mozilla/GTestRunner.cpp
@@ +84,5 @@
>  
>    PR_SetEnv("XPCOM_DEBUG_BREAK=stack-and-abort");
>  
>    ScopedXPCOM xpcom("AsyncPanZoomController");
> +  gfxPrefs::One();

IMO 'One' isn't a good name for 'GetSingleton'. I'd like to see a better name for this function because it makes the call site pretty bad.

Ideally the tests would be able to setup and tear down the state they need to run which would include gfxPrefs. I came imagine this is very impracticable so this is ok.

Comment 4

[Benoit Girard (:BenWa)]

(In reply to Milan Sreckovic [:milan] from comment #1)
> Created attachment 8383295 [details] [diff] [review]
> Initialize gfxPrefs in GTestRunner
> 
> Or, should we initialize this in each individual test? With bug 971943,
> ASync... test needs it.

Ideally yes, but then we'd need to support tear down code for this which is sucky.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Setup and teardown in each test that needs it.

Instead of the global approach, here's the one that does it in each test that needs it.  Less intrusive.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

https://tbpl.mozilla.org/?tree=Try&rev=987f3f533027

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Benoit Girard (:BenWa) from comment #3)
> ...
> IMO 'One' isn't a good name for 'GetSingleton'. I'd like to see a better
> name for this function because it makes the call site pretty bad.

Fair enough.  I'll make that change once I clear a patch queue with related changes.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6111f616591a

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6111f616591a