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)

enhancement
Not set
normal

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
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.
Depends on: 1381525, 1381335
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)
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)
Tests running both as in, running with activity-stream in nightly and tiles in beta etc., if possible.
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.
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)
(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)
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)
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)
Right, and also presumably make the tests run on all channels, not just Nightly.
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 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+
Filed https://github.com/mozilla/activity-stream/issues/2880 to track the test behavior of default true/false and build/not-build.
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……………
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!?
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)
Pushed by edilee@gmail.com: https://hg.mozilla.org/integration/autoland/rev/87e8ccdd8aa6 Turn on Activity Stream by default for Nightly. r=mconley
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
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)
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/07a7f3038e14 Turn on Activity Stream by default for Nightly. r=mconley
Relanded because the failing test had only been automatically moved into a different bucket when this landed.
Flags: needinfo?(mconley)
Flags: needinfo?(edilee)
Depends on: 1381804
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)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Assignee: nobody → edilee
(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)
Depends on: 1382139
Depends on: 1377317
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]
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]
As per Comment 30 and Comment 31, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Depends on: 1382719
Depends on: 1382887
Depends on: 1382079
Depends on: 1383593
Depends on: 1383875
Depends on: 1383921
Depends on: 1384052
No longer depends on: 1383921
Depends on: 1384072
Depends on: 1382367
Depends on: 1401201
Depends on: 1403414
Depends on: 1408026
Depends on: 1438143
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: