1250630 - remove PBackgroundTest and ifdef ENABLE_TEST blocks

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

in bug 1248590, we are working to remove ifdef ENABLE_TESTS, and in comment 6 (and further down) there seems to be support for removing this:
https://bugzilla.mozilla.org/show_bug.cgi?id=1248590#c6

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: MozReview Request: Bug 1250630 - remove PBackgroundTest and ifdef ENABLE_TEST blocks. r?bkelly

Review commit: https://reviewboard.mozilla.org/r/36133/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36133/

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8722617 [details]
MozReview Request: Bug 1250630 - remove PBackgroundTest and ifdef ENABLE_TEST blocks. r?bkelly

Sorry, I'm not a peer on these modules.  I'm also on PTO this week.  Redirecting to Kyle for now.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8722617 [details]
MozReview Request: Bug 1250630 - remove PBackgroundTest and ifdef ENABLE_TEST blocks. r?bkelly

I don't do reviews in mozreview, sorry.  Please upload an attachment and I'll take a look at it.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: remove ENABLE_TESTS from dom/ipc files

and for a simpler review experience...a raw patch

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

this looks good on try server:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f596e9b387c

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8722951 [details] [diff] [review]
remove ENABLE_TESTS from dom/ipc files

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

::: dom/ipc/ContentParent.cpp
@@ -367,5 @@
> -    nsresult rv = observerService->RemoveObserver(this, aTopic);
> -    MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv));
> -
> -    if (!strcmp(aTopic, "profile-after-change")) {
> -      if (mozilla::Preferences::GetBool("pbackground.testing", false)) {

You need to remove pbackground.testing from testing/profiles/prefs_general.js

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/68c323470663

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/68c323470663

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

[Tracking Requested - why for this release]: I would like to uplift this to aurora so firefox 46 can take advantage of build promotion (bug 1248590)

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8722951 [details] [diff] [review]
remove ENABLE_TESTS from dom/ipc files

Approval Request Comment
[Feature/regressing bug #]: 1248590
[User impact if declined]: N/A
[Describe test coverage new/current, TreeHerder]: removing test code which is getting coverage in other places.
[Risks and why]: none that I can think of.
[String/UUID change made/needed]: N/A

Comment 11

[Liz Henry (:lizzard)]

Comment on attachment 8722951 [details] [diff] [review]
remove ENABLE_TESTS from dom/ipc files

Sounds like there is test coverage elsewhere, good idea.

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dd57cfb6f101

Comment 13

[Carsten Book [:Tomcat]]

(In reply to Joel Maher (:jmaher) from comment #12)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/dd57cfb6f101

setting flags