Closed
Bug 997188
Opened 10 years ago
Closed 10 years ago
testsuite connects to telemetry-experiment.cdn.mozilla.net
Categories
(Firefox Health Report Graveyard :: Client: Desktop, defect)
Tracking
(firefox29 unaffected, firefox30 unaffected, firefox31 fixed, firefox-esr24 unaffected)
VERIFIED
FIXED
Firefox 31
Tracking | Status | |
---|---|---|
firefox29 | --- | unaffected |
firefox30 | --- | unaffected |
firefox31 | --- | fixed |
firefox-esr24 | --- | unaffected |
People
(Reporter: froydnj, Assigned: benjamin)
References
Details
(Whiteboard: p=2 s=it-31c-30a-29b.3 [qa-])
Attachments
(1 file)
6.84 KB,
patch
|
Irving
:
review+
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
This behavior is semi-deliberate, according to prefs in testing/profiles/prefs_general.js: // AddonManager tests require that the experiments feature be enabled. user_pref("experiments.enabled", true); user_pref("experiments.supported", true); user_pref("experiments.logging.level", "Trace"); Since the experiments support is enabled globally, we connect to the relevant server at random times, resulting in problems if we disable non-local connections in the testsuite: https://tbpl.mozilla.org/?tree=Try&rev=49e9c7a82979 Why were these prefs enabled for the entire testsuite, rather than just the tests that matter? I see that prefs support picking a different server for the tests. Can we simply pick a different (locally-resolvable) server for the purposes of the testsuite?
Assignee | ||
Comment 1•10 years ago
|
||
gfritzsche is working on cleaning up the mess. This should almost certainly disable experiments by default and enable them just for the browser_experiments browser-chrome test with the local testing manifest. This also is probably the cause of bug 994686.
Assignee: nobody → georg.fritzsche
Blocks: telex
Component: General → Client: Desktop
Product: Core → Firefox Health Report
Comment 2•10 years ago
|
||
The manifest URI will be pointed to a local address here: https://reviewboard.allizom.org/r/63/ But yeah, there is really no reason to enable experiments everywhere - will follow up on this here.
Assignee | ||
Updated•10 years ago
|
Assignee: georg.fritzsche → benjamin
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8407938 -
Flags: review?(irving)
Attachment #8407938 -
Flags: review?(georg.fritzsche)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8407938 [details] [diff] [review] bug997188-telex-disabledintests Review of attachment 8407938 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/profiles/prefs_general.js @@ +52,5 @@ > user_pref("font.size.inflation.minTwips", 0); > > +// Don't accidentally pick up experiments from production. Experiment tests > +// will set the manifest to specific URLs before enabling experiments. > +user_pref("experiments.enabled", false); FWIW, for local testing, I set experiments.enabled and experiments.supported to false and found that I *still* had to set the uri to something local before I stopped hitting mozilla.net. I do not have a good explanation as to why this is.
Assignee | ||
Comment 5•10 years ago
|
||
When did you test this? I believe this was fixed with bug 994727, which landed to m-c on 04-11
Flags: needinfo?(nfroyd)
Reporter | ||
Comment 6•10 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #5) > When did you test this? I believe this was fixed with bug 994727, which > landed to m-c on 04-11 Ah, my working tree is from 04-10. Perhaps that's the problem, then. Carry on!
Flags: needinfo?(nfroyd)
Comment 7•10 years ago
|
||
Comment on attachment 8407938 [details] [diff] [review] bug997188-telex-disabledintests Review of attachment 8407938 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #8407938 -
Flags: review?(georg.fritzsche) → review+
Updated•10 years ago
|
Attachment #8407938 -
Flags: review?(irving) → review+
Updated•10 years ago
|
Flags: firefox-backlog?
Comment 8•10 years ago
|
||
This is fixed by bug 989137, specifically https://reviewboard.allizom.org/r/63/. Disabling the feature via preferences invalidates the effectiveness of tests. It would have received r- from me.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Comment 9•10 years ago
|
||
To more directly answer Nathan's concerns, the pref/service is globally enabled because we want other tests to indirectly test the interaction with this feature/service. If we don't do that, we're not testing real world conditions and our tests could be masking real world failures.
Updated•10 years ago
|
Flags: firefox-backlog?
Updated•10 years ago
|
status-firefox29:
--- → unaffected
status-firefox30:
--- → unaffected
status-firefox31:
--- → fixed
status-firefox-esr24:
--- → unaffected
Reporter | ||
Comment 10•10 years ago
|
||
Nope, not fixed. Try run with m-c from this morning: https://tbpl.mozilla.org/?tree=Try&rev=197a9794cdfb We see connections to the host in question: https://tbpl.mozilla.org/php/getParsedLog.php?id=38097359&tree=Try You can verify that the try push had the appropriate bits from bug 989137 in it: https://hg.mozilla.org/try/file/197a9794cdfb/testing/profiles/prefs_general.js#l58 I don't know what is going wrong here.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Reporter | ||
Updated•10 years ago
|
Updated•10 years ago
|
Flags: firefox-backlog?
Comment 11•10 years ago
|
||
Probably clearUserPref from browser_experiments.js clearing prefs_general.js, especially since it's failing in an unrelated test. I like my solution in bug 995463 to deterministically reset prefs after each test.
Assignee | ||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/122a4087016c I landed this with the following changes relevant to comment 8/10/11: * Don't provide any default for experiments.enabled in prefs_general.js: use whatever the app default is * Don't clearUserPref the URI pref: save the old value and re-set it.
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/122a4087016c
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Updated•10 years ago
|
Updated•10 years ago
|
Flags: firefox-backlog?
Updated•10 years ago
|
Flags: firefox-backlog+
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa?]
Updated•10 years ago
|
QA Contact: kamiljoz
Updated•10 years ago
|
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa?] → p=2 s=it-31c-30a-29b.3 [qa+]
Comment 14•10 years ago
|
||
Hi Kamil, confirming if this resolved bug can be verified before the end of the desktop iteration on Monday April 28?
Flags: needinfo?(kamiljoz)
Comment 15•10 years ago
|
||
Changing this to [qa-] and marking it as verified. This work was done for the automation infrastructure and I'm not sure what I can do here to ensure that it has been fixed. I also double checked with the devs in IRC and made sure there wasn't anything that I can do without involving the automation infrastructure. If someone disagrees, please feel free to re-open and re-assign the ticket a [qa+].
Status: RESOLVED → VERIFIED
Flags: needinfo?(kamiljoz)
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa+] → p=2 s=it-31c-30a-29b.3 [qa-]
Updated•6 years ago
|
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•