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)

x86_64
Linux
defect
Not set
normal

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)

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?
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
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: georg.fritzsche → benjamin
Blocks: 994686
Attachment #8407938 - Flags: review?(irving)
Attachment #8407938 - Flags: review?(georg.fritzsche)
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.
When did you test this? I believe this was fixed with bug 994727, which landed to m-c on 04-11
Flags: needinfo?(nfroyd)
(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 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+
Attachment #8407938 - Flags: review?(irving) → review+
Flags: firefox-backlog?
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
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.
Flags: firefox-backlog?
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 → ---
Flags: firefox-backlog?
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.
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.
https://hg.mozilla.org/mozilla-central/rev/122a4087016c
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Flags: firefox-backlog?
Flags: firefox-backlog+
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa?]
QA Contact: kamiljoz
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa?] → p=2 s=it-31c-30a-29b.3 [qa+]
Hi Kamil, confirming if this resolved bug can be verified before the end of the desktop iteration on Monday April 28?
Flags: needinfo?(kamiljoz)
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-]
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.