1752139 - Support `TestResetFOG` on Android

Status: RESOLVED FIXED

(Toolkit :: Telemetry, task, P3)

Description

[Chris H-C :chutten]

FOG inits on Android in tests, so we should be able to reset FOG in tests, too.

And then we can drop the skip-if in test_GIFFT.js

Comment 1

[Alessio Placitelli [:Dexter]]

(In reply to Chris H-C :chutten from comment #0)

FOG inits on Android in tests, so we should be able to reset FOG in tests, too.

Hey Chris,

I have a couple of questions around this:

1. Do we need to init FOG on Android at all? (in order for us to collect data from Gecko in Fenix for example)
2. If the answer to (1) is yes, why would we need the reset function to work on Android (e.g. why do we have things such as this one?
3. Does testing a metric in m-c "on Android" imply it will work in Fenix?
4. If we really need testResetFOG to work on Android, how much work would this be?

Comment 2

[Chris H-C :chutten]

(In reply to Alessio Placitelli [:Dexter] from comment #1)

1. Do we need to init FOG on Android at all? (in order for us to collect data from Gecko in Fenix for example)

In tests, specifically xpcshell tests which have no "application" code responsible for init'ing Glean, yes.

2. If the answer to (1) is yes, why would we need the reset function to work on Android (e.g. why do we have things such as this one?

Tests need to be able to reset FOG in order that Glean can be reset so that instrumentation tests start from a known state of instrumentation. Or, in short: the usual arguments for testreset apply irrespective of Androidness.

3. Does testing a metric in m-c "on Android" imply it will work in Fenix?

Nope. It implies it works when built for and running on Android under xpcshell. There's some good overlap between that and "works in Fenix", but xpcshell isn't Fenix. (This is the same argument for/against/complicating testing of anything in xpcshell: "is it enough like the applications that the tests are meaningful?")

For the FOG/Glean part... the differences between FOG/Glean-on-xpcshell and FOG/Glean-on-Fenix aren't many, now that Fenix uses gecko's Glean (when available). The biggest difference is that xpcshell is the one that inits Glean via FOG, whereas Fenix inits Glean directly without FOG being init... but that shouldn't matter to any instrumentation being tested: they just care that there is a Glean somewhere. Not who init it.

4. If we really need testResetFOG to work on Android, how much work would this be?

According to Jan-Erik's and my conversation on that subject: not much. Maybe we have an Android-specific one with non-pref-based defaults like Android's test-only fog_init_internal. Or maybe we can make it cross-platform and have Android "just" use the non-Android impl.

Either way it's a matter of trying it out and pushing to try.

Comment 3

[Alessio Placitelli [:Dexter]]

Thank you Chris!

Comment 4

[Chris H-C :chutten]

Remember: part of fixing this test is grepping the source for xpcshell tests that have had to skip Android for lack of testResetFOG. Look for this bug number in folks' comments and skim other uses of testResetFOG.

Comment 5

[Perry McManis [:perry.mcmanis]]

Attachment: WIP: Bug 1752139 - Support `TestResetFOG` on Android r?chutten

Depends on D179612

Comment 6

[Perry McManis [:perry.mcmanis]]

Attachment: WIP Bug 1752139 - Support `TestResetFOG` on Android r?chutten

Comment 7

[Perry McManis [:perry.mcmanis]]

Attachment: Bug 1752139 - Support `TestResetFOG` on Android r?chutten

Depends on D179612

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07a0dce54c55
Support `TestResetFOG` on Android r=chutten,extension-reviewers,cookie-reviewers,pbz,robwu

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/07a0dce54c55

Comment 10

[Cristian Tuns]

Backed out for possibly causing Bug 1837869
Backout link: https://hg.mozilla.org/mozilla-central/rev/c313d499bd6e530a5e15cf3cf43e129650bcd09e

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/9cf6f4016745
Support `TestResetFOG` on Android r=chutten,extension-reviewers,cookie-reviewers,pbz,robwu

Comment 12

[Cristian Tuns]

It looks like this was not the cause of Bug 1837869 and it got relanded.