Closed Bug 1250630 Opened 4 years ago Closed 4 years ago

remove PBackgroundTest and ifdef ENABLE_TEST blocks

Categories

(Core :: DOM: Content Processes, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 - fixed
firefox47 --- fixed

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

Attachments

(1 file, 1 obsolete file)

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 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.
Attachment #8722617 - Flags: review?(bkelly) → review?(khuey)
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.
Attachment #8722617 - Flags: review?(khuey) → review-
and for a simpler review experience...a raw patch
Assignee: nobody → jmaher
Attachment #8722617 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8722951 - Flags: review?(khuey)
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
Attachment #8722951 - Flags: review?(khuey) → review+
https://hg.mozilla.org/mozilla-central/rev/68c323470663
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
[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 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
Attachment #8722951 - Flags: approval-mozilla-aurora?
Comment on attachment 8722951 [details] [diff] [review]
remove ENABLE_TESTS from dom/ipc files

Sounds like there is test coverage elsewhere, good idea.
Attachment #8722951 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.