59 bytes, text/x-review-board-request
+++ This bug was initially created as a clone of Bug #1349348 +++ Major feature of this sync is the initial implementation of Preference Experiments. PRs since last sync (Commit PR# - Description): dc28b5d PR #713 - Merge preference-experiments into master 44a7721 PR #684 - Watch experiment prefs 8009444 PR #702 - [#636] Update DRF to avoide Crashes c7ebb9e PR #673 - Add pref experiment type checks faba71f PR #700 - Merge master into preference-experiment 46ac267 PR #697 - recipe-client-addon: Convert PreferenceExperiments tests to mochitest. d87b9bd PR #699 - #698: Treat malformed lastShown dates as valid in show-heartbeat. 966030c PR #696 - Fix mach lint error for useless parameter to addObserver. a234dd6 PR #692 - Merge preference experiments into master cef4cb4 PR #693 - Refactor TaskCluster tasks and fix linting / tests 0386d6a PR #694 - Backport preference fix 61a024a PR #691 - Do not break `this` in functions wrapped with SandboxManager.wrapAsync. 4f0ee5d PR #690 - Re-add preprocessor directive and modify make-xpi.sh to strip it. 984da2d PR #683 - Bump TC max run time to 8 hours and deadline to 1 day. 7645206 PR #681 - Inform telemetry pt 2 cef2b33 PR #659 - Stop preference experiments when the server doesn't send them c658d82 PR #679 - Fallback for LocaleService::GetAppLocale. f4ec9f9 PR #686 - recipe-server: Update signatures when closing approval requests. 72fd339 PR #688 - recipe-server/contract-tests: Skip cache tests on api URLs. e41c37b PR #685 - Merge master into preference-experiments branch 08a649e PR #677 - Revert "#633 from andymikulski/433-addon-storage-durability" c260972 PR #678 - Remove comment from install.rdf.in. 86c8973 PR #670 - Fix #652: Add option for setting user or default prefs for experiments. 7a4afa0 PR #641 - recipe-client-addon: Use nsIUpdateTimerManager to run every 24 hours f2e2775 PR #676 - Merge master into preference-experiments branch a4015a0 PR #682 - recipe-server: Add test for signing consistency during approval. cfb2e45 PR #671 - Update locales/channel/countries with recipe API f066acc PR #666 - recipe-client-addon: Separate build and test in Taskcluster CI f9f659a PR #672 - recipe-server: Update Django, 1.10.5 -> 1.10.7 590ccc2 PR #658 - Fix ordering on recipe history 68dce0e PR #660 - Fix #612: Add ability to filter recipes by active and expired experiments. 4595abb PR #651 - recipe-server: Add per-action validation of recipe arguments bb68434 PR #661 - #351 Add Preference filter expressions 2d10f37 PR #662 - recipe-client-addon: Use getAppLocaleAsLangTag for getting browser locale 3e2231f PR #657 - recipe-client-addon: Calculate add-on versions automatically from git tags 1e0cafd PR #655 - taskcluster: Use custom base image for TC builds 5513601 PR #649 - recipe-client-addon: Unconditionally set up logging in bootstrap deff7bd PR #633 - addon: Add Storage durability checks (#433) 606a355 PR #656 - recipe-server: Add migration to remove recipe signatures again 4c8401c PR #613 - Merge peer-signing feature branch into master 5279341 PR #648 - Update pytest-testrail to latest version 99aaf0d PR #630 - Updated documentation for running the contract API tests 99710cf PR #654 - Merge master into preference-experiment 22e714a PR #643 - Describe ratioSample in a simpler way, and use async/await for it. 8325105 PR #653 - Remove bucketCount argument from preference experiments. 261f597 PR #642 - Add normandy.recipe to filter expressions in the add-on. b15294a PR #644 - Remove functional tests from CI 6b4e984 PR #639 - #587: Add Sampling.ratioSample and ratio branching for preference-experiments 6e96cac PR #634 - Bug 1349576: Remove use of services.sync.numClients preference. c7013f2 PR #623 - Preference experiment driver API 334d902 PR #629 - Bug 1349596: Validate JEXL on the server instead of the client. bf95e3e PR #632 - Fix TaskCluster builds. 2172b15 PR #628 - Bug 1348885: Do not call Cu.reportError on errors from the sandbox. 990e830 PR #626 - recipe-server: Use more eslint rules from eslint-plugin-react 44cb46c PR #603 - Basic preference experiment action 06b91fd PR #620 - recipe-server: Handle validation errors in API gracefully 7bbc9c3 PR #624 - Refactor all uses of Task.jsm to use native async/await instead. 9912541 PR #573 - Revision history lookup fix 4566d03 PR #571 - Update state tree correctly on status change a13c996 PR #567 - Approval/Rejection comments e0124e8 PR #558 - #490 Revision status indicator 19f2adc PR #559 - Revise `arguments` correctly ec48aa4 PR #561 - Implement Enable/Disable button 8b65f4a PR #562 - Add `revision` property to Recipes store a0e1f4b PR #554 - Add basic UI for Review Requests ff8ac52 PR #551 - Recipe status workflow e2ed691 PR #550 - Add User API endpoint and current user id to base template f3d244c PR #518 - Document approval request workflow and API b954377 PR #516 - Include the approval request with the serialized recipe eb9f71e PR #502 - Change from `update` to `revise` 9869231 PR #498 - Add comment field to ApprovalRequest 814b560 PR #438 - Approval requests
Okay, I think I did the thing correctly this time. I mostly guessed at the tests and platforms I wanted for the pre-merge try run. https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf86a39cdddd3f8915d4fcbf4f185b453e2c2275
Try run failed due to a build error. I'm working on it in a GH issue: https://github.com/mozilla/normandy/issues/718
Comment on attachment 8861716 [details] Bug 1359655 - Sync shield-recipe-client from GitHub (commit e37261c) https://reviewboard.mozilla.org/r/133698/#review136828 I'm swamped and it looks like we'll need to figure out the bootstrap.js thing before proceeding, so punting on this review for now. Though - I assume this is basically just a wrap-up of stuff I've already reviewed on github? In which case this is effectively going to be a rubberstamp review - no point looking at it all again...
Attachment #8861716 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs from comment #5) > Though - I assume this is basically just a wrap-up of stuff I've already > reviewed on github? In which case this is effectively going to be a > rubberstamp review - no point looking at it all again... Correct.
#718 landed, and I've updated the patch with two more merges: e37261c PR #721 - Fix try builds of the system add-on d9331ba PR #714 - recipe-client-addon: Lazy load applicable modules in RecipeRunner
Summary: Sync shield-recipe-client from GitHub (commit dc28b5d ) → Sync shield-recipe-client from GitHub (commit e37261c)
Try run of the new commit (before I tagged it in Github): https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb8e33224b05bbb58ed44afa6daf5054cd94f9b6&selectedJob=94920847&group_state=expanded Failures seem intermittent, I think?
Comment on attachment 8861716 [details] Bug 1359655 - Sync shield-recipe-client from GitHub (commit e37261c) https://reviewboard.mozilla.org/r/133698/#review137470 rs=me. Yeah, the trypush looks fine at a glance. Thanks!
Attachment #8861716 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/ab35d61ad843 Sync shield-recipe-client from GitHub (commit e37261c) r=Gijs
Backed out in https://hg.mozilla.org/integration/autoland/rev/130d91a785a7 for Windows opt and debug, 7 and 8, e10s download panel timeouts, https://treeherder.mozilla.org/logviewer.html#?job_id=94998079&repo=autoland
philor: Reading bug 1352792, it looks like this is an intermittent failure that becomes permanently failed with our patch, as well as with another seemingly-unrelated patch (bug 1320994). Is it possible to disable the test until someone can fix it, especially given the comments in the test code about working around intermittent failures? I have no idea what our guidelines around disabling tests are. If not, are there other options besides politely bugging Paolo or others to fix it so we can land? I could try my hand at it but I'm not at all familiar with the code that's failing.
I disabled it, since the only thing you and Andreas were doing was causing it to move to a different chunk of browser-chrome, and whatever bizarre reason it has for failing in one chunk and not another, not being willing to run in whatever chunk you're assigned is a test party foul. You should be fine to reland once that disabling patch sticks.
Awesome, thanks for the help!
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/53a6c994705a Sync shield-recipe-client from GitHub (commit e37261c) r=Gijs
Comment on attachment 8861716 [details] Bug 1359655 - Sync shield-recipe-client from GitHub (commit e37261c) Approval Request Comment [Feature/Bug causing the regression]: Ability to run preference experiments in Beta/Release 54. [User impact if declined]: We will be unable to test Quantum changes and other features planning to run preference experiments (such as Screenshots) in release until a system add-on update is released via Balrog. In either case we plan to push an update, whether it's via Balrog or via uplift, and uplift does not require us to build a new XPI and get it signed and tested on Balrog. In both cases we have a good method to revert due to being a system add-on, so the only real difference between the two is extra effort (any any consequences of uplift that I'm not aware of), so the decision to request uplift is based mostly on that. [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: Yes, already has approval: https://mail.mozilla.org/pipermail/dev-shield/2017-June/000233.html Note that the email mentions a recommendation to ride 55. We had previously agreed with QE to uplift to Beta, so there's some confusion that needs to be resolved. I'm going to talk with QE tonight about it when they get online, but in the meantime I think delaying merging another day will be worse than requesting uplift now. [List of other uplifts needed for the feature/fix]: Just this patch. [Is the change risky?]: Not really. [Why is the change risky/not risky?]: It's been in Nightly for several weeks, and we've run preference experiments with no major client-related issues (we have hit a few issues due to the Normandy service, which have been resolved and are unrelated to the client). In addition, as a system add-on, if we notice breakage in release, we can push an update via Balrog without doing a new Firefox release to revert the code to the previous version of the add-on quickly. [String changes made/needed]: None
Comment on attachment 8861716 [details] Bug 1359655 - Sync shield-recipe-client from GitHub (commit e37261c) The urgency driving the uplift request was running a preference experiment to test Firefox Screenshots, which has changed it's timeline to test and ship in 55 instead of 54. This means that riding the 55 train will work fine for us, so I'm dropping the uplift request. Sorry for the confusion!
You need to log in before you can comment on or make changes to this bug.