Closed Bug 1385177 Opened 7 years ago Closed 7 years ago

remove check for built-in self-support

Categories

(Firefox :: Normandy Client, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox55 + fixed
firefox56 --- fixed

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(1 file)

In bug 1383338 unfortunately I didn't include the removal of self-support, which is required for that change to be effective.
I understand that we want this in 55 for some new user retention studies.
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
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?
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)
These failures seem really unlikely to be caused by this patch... I'll do a try run.
Flags: needinfo?(rhelmer)
(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 on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client

https://reviewboard.mozilla.org/r/162402/#review167990
Looks like this is a one-line fix to a test config... doing a try run to make sure before re-landing.
(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.
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
Backout by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/392203e46397
Backed out changeset 7a3c198ec229 for breaking PGO
(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.
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
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7f4229cbc665
Followup to fix bustage a=bustage
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)
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)
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
https://hg.mozilla.org/mozilla-central/rev/a15f665d03f4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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+
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.