Closed
Bug 1385177
Opened 7 years ago
Closed 7 years ago
remove check for built-in self-support
Categories
(Firefox :: Normandy Client, defect)
Firefox
Normandy Client
Tracking
()
RESOLVED
FIXED
People
(Reporter: rhelmer, Assigned: rhelmer)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
mythmon
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
In bug 1383338 unfortunately I didn't include the removal of self-support, which is required for that change to be effective.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
I understand that we want this in 55 for some new user retention studies.
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client
https://reviewboard.mozilla.org/r/162402/#review167850
Attachment #8891190 -
Flags: review?(mcooper) → review+
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3b9509ef04d0
Remove check for built-in self-support from shield recipe client r=mythmon
Assignee | ||
Comment 5•7 years ago
|
||
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client
Approval Request Comment
[Feature/Bug causing the regression]: followup for bug 1383338
[User impact if declined]: not able to run first-run retention experiments
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: yes, coordinating w/ QA now and steps will be provided in bug 1383338
[List of other uplifts needed for the feature/fix]: just bug 1383338 (already landed on beta)
[Is the change risky?]: no
[Why is the change risky/not risky?]: removes detection for obsolete "self-support" feature which was causing shield to exit early on first-run
[String changes made/needed]: none
Attachment #8891190 -
Flags: approval-mozilla-beta?
Comment 6•7 years ago
|
||
Backed out on suspicion of causing exceptions in browser-chrome tests and connections in wpt tests:
https://hg.mozilla.org/integration/autoland/rev/50ce68c9412ce80cf4bf4f1dc4be9f3c7fcd0c6d
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3b9509ef04d0c8e87a59aa1b03e4a0beca12f1dc&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure logs browser-chrome:
https://treeherder.mozilla.org/logviewer.html#?job_id=119045097&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=119044063&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=119044015&repo=autoland
E.g. browser/components/extensions/test/browser/browser_ext_windows_update.js | A promise chain failed to handle a rejection: JSON.parse: unexpected character at line 1 column 1 of the JSON data - stack: null Rejection date: Fri Jul 28 2017 11:03:20 GMT-0700 (PDT) - false == true
Failure log wpt: https://treeherder.mozilla.org/logviewer.html#?job_id=119054649&repo=autoland
FATAL ERROR: Non-local network connections are disabled and a connection attempt to example.com (93.184.216.34) was made.
Flags: needinfo?(rhelmer)
Assignee | ||
Comment 7•7 years ago
|
||
These failures seem really unlikely to be caused by this patch... I'll do a try run.
Flags: needinfo?(rhelmer)
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #6)
> Backed out on suspicion of causing exceptions in browser-chrome tests and
> connections in wpt tests:
>
> https://hg.mozilla.org/integration/autoland/rev/
> 50ce68c9412ce80cf4bf4f1dc4be9f3c7fcd0c6d
>
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=3b9509ef04d0c8e87a59aa1b03e4a0beca12f1dc&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=retry&filter-
> resultStatus=usercancel&filter-resultStatus=runnable
>
> Failure logs browser-chrome:
> https://treeherder.mozilla.org/logviewer.html#?job_id=119045097&repo=autoland
> https://treeherder.mozilla.org/logviewer.html#?job_id=119044063&repo=autoland
> https://treeherder.mozilla.org/logviewer.html#?job_id=119044015&repo=autoland
> E.g.
> browser/components/extensions/test/browser/browser_ext_windows_update.js | A
> promise chain failed to handle a rejection: JSON.parse: unexpected character
> at line 1 column 1 of the JSON data - stack: null Rejection date: Fri Jul 28
> 2017 11:03:20 GMT-0700 (PDT) - false == true
I can't reproduce this at all. Maybe we got an unlucky revision from m-c; I'll rebase and do a try run.
> Failure log wpt:
> https://treeherder.mozilla.org/logviewer.html#?job_id=119054649&repo=autoland
> FATAL ERROR: Non-local network connections are disabled and a connection
> attempt to example.com (93.184.216.34) was made.
This appears to be an existing bug - before this patch, shield always exited early on first-run.
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client
https://reviewboard.mozilla.org/r/162402/#review167990
Assignee | ||
Comment 11•7 years ago
|
||
Looks like this is a one-line fix to a test config... doing a try run to make sure before re-landing.
Assignee | ||
Comment 12•7 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #8)
> (In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on
> intermittent or backout) from comment #6)
> > Backed out on suspicion of causing exceptions in browser-chrome tests and
> > connections in wpt tests:
> >
> > https://hg.mozilla.org/integration/autoland/rev/
> > 50ce68c9412ce80cf4bf4f1dc4be9f3c7fcd0c6d
> >
> > Push with failures:
> > https://treeherder.mozilla.org/#/
> > jobs?repo=autoland&revision=3b9509ef04d0c8e87a59aa1b03e4a0beca12f1dc&filter-
> > resultStatus=testfailed&filter-resultStatus=busted&filter-
> > resultStatus=exception&filter-resultStatus=retry&filter-
> > resultStatus=usercancel&filter-resultStatus=runnable
> >
> > Failure logs browser-chrome:
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119045097&repo=autoland
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119044063&repo=autoland
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119044015&repo=autoland
> > E.g.
> > browser/components/extensions/test/browser/browser_ext_windows_update.js | A
> > promise chain failed to handle a rejection: JSON.parse: unexpected character
> > at line 1 column 1 of the JSON data - stack: null Rejection date: Fri Jul 28
> > 2017 11:03:20 GMT-0700 (PDT) - false == true
>
> I can't reproduce this at all. Maybe we got an unlucky revision from m-c;
> I'll rebase and do a try run.
Still can't repro this locally or on try (also I meant to say mozilla-incoming above not m-c)
> > Failure log wpt:
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119054649&repo=autoland
> > FATAL ERROR: Non-local network connections are disabled and a connection
> > attempt to example.com (93.184.216.34) was made.
>
> This appears to be an existing bug - before this patch, shield always exited
> early on first-run.
With the one-line test config fix, I can't repro this either.
Comment 13•7 years ago
|
||
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a3c198ec229
Remove check for built-in self-support from shield recipe client r=mythmon
Comment 14•7 years ago
|
||
Backout by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/392203e46397
Backed out changeset 7a3c198ec229 for breaking PGO
Assignee | ||
Comment 15•7 years ago
|
||
(In reply to Pulsebot from comment #14)
> Backout by gszorc@mozilla.com:
> https://hg.mozilla.org/integration/autoland/rev/392203e46397
> Backed out changeset 7a3c198ec229 for breaking PGO
Ugh. This is a minor problem in the one-line patch... example.com should be replaced with %(server)s not %(server) in the test config.
Comment hidden (mozreview-request) |
Comment 17•7 years ago
|
||
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0aaf1e3ab6ea
Remove check for built-in self-support from shield recipe client r=mythmon
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 20•7 years ago
|
||
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7f4229cbc665
Followup to fix bustage a=bustage
Comment 21•7 years ago
|
||
Backed out for failing browser-chrome's browser_sanitizeDialog.js and browser_clientAuth_connection.js:
https://hg.mozilla.org/integration/autoland/rev/b9abf4de15297fa10e1ee09c99713a2c6286fd69
https://hg.mozilla.org/integration/autoland/rev/1aa98e99c8792d7d76b0069319c51f8f5004690f
Follow-up push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7f4229cbc66563ced5b3277f58fc30295cfeb685&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log sanitize: https://treeherder.mozilla.org/logviewer.html#?job_id=119196158&repo=autoland
> FAIL | browser/base/content/test/general/browser_sanitizeDialog.js | A promise chain failed to handle a rejection: NetworkError when attempting to fetch resource. - stack: open@chrome://mochitests/content/browser/browser/base/content/test/general/browser_sanitizeDialog.js:822:5 test_cannot_clear_history@chrome://mochitests/content/browser/browser/base/content/test/general/browser_sanitizeDialog.js:379:3 Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:798:21 TaskImpl_run@resource://gre/modules/Task.jsm:331:42 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 TaskImpl@resource://gre/modules/Task.jsm:280:3 asyncFunction@resource://gre/modules/Task.jsm:252:14 Task_spawn@resource://gre/modules/Task.jsm:166:12 Tester_execTest@chrome://mochikit/content/browser-test.js:789:9 Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:689:7 SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59 Rejection date: Sat Jul 29 2017 06:36:51 GMT+0000 (Coordinated Universal Time) - false == true
Failure log auth: https://treeherder.mozilla.org/logviewer.html#?job_id=119196171&repo=autoland
> FAIL | security/manager/ssl/tests/mochitest/browser/browser_clientAuth_connection.js | A promise chain failed to handle a rejection: NetworkError when attempting to fetch resource. - stack: null Rejection date: Sat Jul 29 2017 06:44:09 GMT+0000 (Coordinated Universal Time) - false == true
Flags: needinfo?(rhelmer)
Assignee | ||
Comment 22•7 years ago
|
||
Thanks. What's going on here is the test config is pointing to the local test server, and a few other tests in the tree aren't expecting to see this connection.
Reviewing the shield tests, they just expect the pref to be present but they actually set it themselves.
Flags: needinfo?(rhelmer)
Comment hidden (mozreview-request) |
Comment 24•7 years ago
|
||
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a15f665d03f4
Remove check for built-in self-support from shield recipe client r=mythmon
Comment 25•7 years ago
|
||
bugherder |
Comment 26•7 years ago
|
||
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client
shield fix for 55 rc1
Attachment #8891190 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 27•7 years ago
|
||
bugherder uplift |
Updated•7 years ago
|
Product: Shield → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•