Closed
Bug 1381569
Opened 7 years ago
Closed 7 years ago
Turn on Activity Stream by default for Nightly
Categories
(Firefox :: Activity Streams: General, enhancement)
Firefox
Activity Streams: General
Tracking
()
VERIFIED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | verified |
People
(Reporter: Mardak, Assigned: Mardak)
References
(Depends on 2 open bugs)
Details
Attachments
(1 file)
We'll need to match the building of activity-stream as we don't want to be enabled in Firefox but not actually have the content to load:
https://searchfox.org/mozilla-central/source/browser/extensions/moz.build#39-42
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
This can't land until bug 1381335 and bug 1381525 make it to mozilla-central (or I suppose technically on autoland, where they currently are, if this patch also lands on autoland). Doesn't make sense to push to try unless those other commits are included.
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 8887154 [details]
Bug 1381569 - Turn on Activity Stream by default for Nightly.
This patch turns on activity stream in nightly, but what should happen outside of nightly? I.e., should our tests be running in beta/release if the extension isn't being built?
On one hand, it could be good to test the expected behavior with activity-stream enabled and disabled by having our tests run (where load_location only passes with enabled=true).
On the other hand, as we add additional tests, making sure they all pass when enabled AND disabled can be tricky.
Who should r? the enable patch? mconley?
Flags: needinfo?(khudson)
Flags: needinfo?(dmose)
Comment 4•7 years ago
|
||
I would defer to whatever other similar roll-out strategies have done, but I think it would be a good idea to have tests running with both for now since we're eventually planning on doing a staged roll-out in release.
I also think mconley would be a good reviewer so he can share in the joy.
Flags: needinfo?(khudson)
Comment 5•7 years ago
|
||
Tests running both as in, running with activity-stream in nightly and tiles in beta etc., if possible.
Assignee | ||
Comment 6•7 years ago
|
||
Tiles tests will be running in nightly and beta, etc, and this is supporting activity-stream enabled/disabled. The current patch has activity-stream tests only running in nightly and only supports activity-stream enabled.
Assignee | ||
Comment 7•7 years ago
|
||
Comment 8•7 years ago
|
||
As far as what existing practice is for this kind of thing, maybe Corey Price would know? Or someone from PI (eg bsmedberg)
(In reply to Ed Lee :Mardak from comment #6)
> Tiles tests will be running in nightly and beta, etc, and this is supporting
> activity-stream enabled/disabled. The current patch has activity-stream
> tests only running in nightly and only supports activity-stream enabled.
Can you unpack that first sentence somewhat? I'm having a hard time figuring out how it's not in conflict with the second sentence.
Flags: needinfo?(dmose) → needinfo?(edilee)
Assignee | ||
Comment 9•7 years ago
|
||
(In reply to Dan Mosedale (:dmose) from comment #8)
> Can you unpack that first sentence somewhat? I'm having a hard time
> figuring out how it's not in conflict with the second sentence.
browser/base/content/test/newtab tests pass whether activity-stream.enabled is true or false (because head.js forces the pref to false).
browser/extensions/activity-stream/test/functional/mochitest only passes when activity-stream.enabled is true.
The newtab tests are currently running on nightly, beta, etc.
The activity-stream tests are not running anywhere except our pine/try builds when we modify moz.build to have BROWSER_CHROME_MANIFESTS += ['test/functional/mochitest/browser.ini']. But this patch will turn it on in nightly but keeps them only running nightly.
Flags: needinfo?(edilee)
Comment 10•7 years ago
|
||
Would having the activity-stream test behavior match the tiles test behavior be the win here? This way they're effectively each running with the pref value that they're expected to have
> On the other hand, as we add additional tests, making sure they all pass when enabled AND disabled can be tricky.
Where does the trickiness come in? Since we'd be forcing activity-stream on for the activity-stream tests, even if the build had things disabled, we'd end up with (effectively) a single environment...
Flags: needinfo?(edilee)
Assignee | ||
Comment 11•7 years ago
|
||
Oh, I suppose we could add a head.js to force activity-stream on. And maybe make the load_location test with the pref enabled and disabled as a sanity check (even though tiles newtab would be checking it anyway).
Flags: needinfo?(edilee)
Comment 12•7 years ago
|
||
Right, and also presumably make the tests run on all channels, not just Nightly.
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8887154 [details]
Bug 1381569 - Turn on Activity Stream by default for Nightly.
This patch turns on activity stream in nightly as well as its related tests. We'll have a followup issue to clean up activity-stream test coverage to make sure the transition from enabled (nightly) -> disabled (beta) doesn't cause additional test failures.
Perfherder looks good:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=e0b0865639cebc1b5afa0268a4b073fcdde0e69c&newProject=try&newRevision=790f8426d1641c96bdccce74f7f0c221fdfa5e00&framework=1
Treeherder looks good except windows 10 debug:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=790f8426d1641c96bdccce74f7f0c221fdfa5e00&group_state=expanded&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified
Just making sure, we will need to make sure there aren't test regressions on Windows 10 debug? (I retriggered a bunch to see if they're perma or intermittent.)
Attachment #8887154 -
Flags: review?(mconley)
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8887154 [details]
Bug 1381569 - Turn on Activity Stream by default for Nightly.
https://reviewboard.mozilla.org/r/157898/#review163140
Looks good! Assuming those failures on try are indeed intermittent, I say we land this puppy.
Might be worth a firefox-dev mailing list post as well.
Attachment #8887154 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 15•7 years ago
|
||
Filed https://github.com/mozilla/activity-stream/issues/2880 to track the test behavior of default true/false and build/not-build.
Assignee | ||
Comment 16•7 years ago
|
||
Unfortunately it looks like those Windows 10 x64 debug tests are mostly perma-orange:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=728274b408b41fa6e5fdb4070fe099357ecf69f7
Tracking them here:
https://github.com/mozilla/activity-stream/projects/15
Seems like we missed these as our `pine` pushes test with Windows x64 on Windows 8 instead of 10……………
Assignee | ||
Comment 17•7 years ago
|
||
Oh wait, I thought I remembered looking at this, and that's why I was asking if Windows 10 x64 debug was important, but I seemed to have confused myself in the process somewhere.
Those same tests are failing even without activity stream enabled:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=728274b408b41fa6e5fdb4070fe099357ecf69f7&selectedJob=115021514
I think I searched for "windows 10 x64 debug browser-chrome" on mozilla-central treeherder and too quickly glanced over the 8 vs 10 assuming the passing tests were for Windows 10, but they were actually only run on Windows 8!
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=windows%2010%20x64%20debug%20browser-chrome
I'M LANDING THIS!?
Comment 18•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s 36668b53d2aa -d b8f66d5a9748: rebasing 407525:36668b53d2aa "Bug 1381569 - Turn on Activity Stream by default for Nightly. r=mconley" (tip)
merging browser/extensions/activity-stream/test/functional/mochitest/browser.ini
warning: conflicts while merging browser/extensions/activity-stream/test/functional/mochitest/browser.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment 19•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment 21•7 years ago
|
||
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/87e8ccdd8aa6
Turn on Activity Stream by default for Nightly. r=mconley
Comment 22•7 years ago
|
||
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/b9031e50fd76
Backed out changeset 87e8ccdd8aa6 for failing browser/base/content/test/performance/browser_windowopen_reflows.js on Windows 8 x64. r=backout
Comment 23•7 years ago
|
||
Backed out
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=87e8ccdd8aa6f6b549922b4debc19a4126b7d940&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=115179839&repo=autoland
03:51:22 INFO - TEST-START | browser/base/content/test/performance/browser_windowopen_reflows.js
03:51:22 INFO - TEST-INFO | started process screenshot
03:51:22 INFO - TEST-INFO | screenshot: exit 0
03:51:22 INFO - Buffered messages logged at 03:51:22
03:51:22 INFO - Entering test bound
03:51:22 INFO - Skipping this test because of perma-failures on Windows 8 x64 (bug 1381521)
03:51:22 INFO - Leaving test bound
03:51:22 INFO - Buffered messages finished
03:51:22 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_windowopen_reflows.js | This test contains no passes, no fails and no todos. Maybe it threw a silent exception? Make sure you use waitForExplicitFinish() if you need it. -
03:51:22 INFO - GECKO(3368) | MEMORY STAT | vsize 1875MB | vsizeMaxContiguous 6839480MB | residentFast 350MB | heapAllocated 141MB
03:51:22 INFO - TEST-OK | browser/base/content/test/performance/browser_windowopen_reflows.js | took 22ms
Mike, can you take a look at this, please? Seems the check from bug 1363361 doesn't work anymore.
Flags: needinfo?(mconley)
Flags: needinfo?(edilee)
Comment 24•7 years ago
|
||
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/07a7f3038e14
Turn on Activity Stream by default for Nightly. r=mconley
Comment 25•7 years ago
|
||
Relanded because the failing test had only been automatically moved into a different bucket when this landed.
Flags: needinfo?(mconley)
Flags: needinfo?(edilee)
Comment 26•7 years ago
|
||
Hey Mardak,
Weren't we supposed to also turn off the content process preloader at the same time as turning Activity Stream on?
Flags: needinfo?(edilee)
Comment 27•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•7 years ago
|
Assignee: nobody → edilee
Assignee | ||
Comment 28•7 years ago
|
||
(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #26)
> Weren't we supposed to also turn off the content process preloader at the
> same time as turning Activity Stream on?
I believe the discussion originated from trying to fix bug 1372664, and when we tried flipping dom.ipc.processPrelaunch.enabled, it didn't help because that specific test was already setting prelaunch to false. The longer term fix discussed resulted in bug 1376895 being filed. I can flip the pref in bug 1381804 if you think it'll help with the memory regression?
Flags: needinfo?(edilee)
Comment 30•7 years ago
|
||
I have reproduced this bug with Nightly 56.0a1 (2017-07-17) on Ubuntu 16.04, 64 bit!
The fix is now verified on Latest Nightly.
Build ID 20170719100147
User Agent Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170719]
Comment 31•7 years ago
|
||
I have successfully reproduced this bug with Nightly 56.0a1 (2017-07-17) (32-bit) on windows 10(32bit)
this bug is verified fix with latest nightly 56.0a1 (2017-07-19) (32-bit)
Build ID: 20170719030206
Mozilla/5.0 (Windows NT 10.0; rv:56.0) Gecko/20100101 Firefox/56.0
[bugday-20170719]
Comment 32•7 years ago
|
||
As per Comment 30 and Comment 31, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•