mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.49k stars 1.27k forks source link

Intermittent UI test failure - Some tests depend on external sites which can cause intermittent or perma failures #23521

Closed jonalmeida closed 1 year ago

jonalmeida commented 2 years ago

Some UI tests run against websites that are external to the test framework. This means that reproducible builds are not possible when the external site itself has changed.

During an uplift (see https://github.com/mozilla-mobile/fenix/pull/23515 and https://github.com/mozilla-mobile/fenix/pull/23510), we were seeing perma-fail UI tests for mainMenuInstallPWATest because the text assertions needed to be updated. The test failures also gave the impression of being unrelated test failures which can be more harmful if left unfixed.

While the test was fixed on main in #23335, this would fix the tests for all the previous releases.

Have we considered investigating if we can run a local webserver on the device with the web site contents that we can make assertions against? OkHttp has a MockWebServer that we could consider for serving up the site we want to test against that we've used for unit tests. To clarify, I don't think a solution is to just uplift UI tests to the active branches that we are releasing, but rather have one contained test suite that we can always go back to on a commit-by-commit basis and have the UI tests pass.

Firebase Test Run:

https://console.firebase.google.com/project/moz-fenix/testlab/histories/bh.66b7091e15d53d45/matrices/8218237416948585934

Stacktrace:

androidx.test.uiautomator.UiObjectNotFoundException: UiSelector[TEXT=yay app]
at androidx.test.uiautomator.UiObject.clickAndWaitForNewWindow(UiObject.java:456)
at org.mozilla.fenix.ui.robots.AddToHomeScreenRobot$Transition.openHomeScreenShortcut(AddToHomeScreenRobot.kt:64)
at org.mozilla.fenix.ui.SmokeTest.mainMenuInstallPWATest(SmokeTest.kt:1113)
at java.lang.reflect.Method.invoke(Native Method)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at androidx.test.internal.runner.junit4.statement.RunBefores.evaluate(RunBefores.java:80)
at androidx.test.internal.runner.junit4.statement.RunAfters.evaluate(RunAfters.java:61)
at androidx.test.rule.ActivityTestRule$ActivityStatement.evaluate(ActivityTestRule.java:531)
at androidx.test.rule.ActivityTestRule$ActivityStatement.evaluate(ActivityTestRule.java:531)
at androidx.compose.ui.test.junit4.AndroidComposeTestRule$AndroidComposeStatement.evaluateInner(AndroidComposeTestRule.android.kt:357)
at androidx.compose.ui.test.junit4.AndroidComposeTestRule$AndroidComposeStatement.evaluate(AndroidComposeTestRule.android.kt:346)
at androidx.compose.ui.test.junit4.android.EspressoLink$getStatementFor$1.evaluate(EspressoLink.android.kt:63)
at androidx.compose.ui.test.junit4.IdlingResourceRegistry$getStatementFor$1.evaluate(IdlingResourceRegistry.jvm.kt:160)
at androidx.compose.ui.test.junit4.android.ComposeRootRegistry$getStatementFor$1.evaluate(ComposeRootRegistry.android.kt:150)
at org.junit.rules.RunRules.evaluate(RunRules.java:20) 

Build:

https://github.com/mozilla-mobile/fenix/pull/23515

cc: @rpappalax for discussion, maybe?

┆Issue is synchronized with this Jira Task

jonalmeida commented 2 years ago

@kbrosnan suggested this web extension to generate single file versions of the test sites that run against.

jonalmeida commented 2 years ago

Looks like we have support for doing this via TestAssetHelper.kt, so maybe the only thing that needs to be done is re-writing the test to use a local version.

rpappalax commented 2 years ago

@jonalmeida thanks for bringing this to our attention. We do already make use of the http3 mock web server to run a number of tests using assets served via localhost so it's probably just a matter of bringing the test page in question into the project as an asset. We do also have a few other remote assets we're trying to bring in to run locally so they're not as brittle, though we can't do that in every single case. @AaronMT if this test is causing perma-failures, can you pls disable it for now until we can bring it in?

jonalmeida commented 2 years ago

@AaronMT if this test is causing perma-failures, can you pls disable it for now until we can bring it in?

We have fixed them on those release branches (see links above) as part of getting the patch releases out for the immediate case so there is nothing to disable.

AaronMT commented 2 years ago

Thanks @jonalmeida, for the most part we do use locally served assets depending on tests, but in others we use remote assets (it depends on the target test). There were complications and concerns with testing local assets is bundling large binaries with the repository (e.g, a large download binary download test), or others such as Enhanced Tracking Protection can not be tested without touching network (as far as I'm aware). We use a small test app web-page hosted on Github to serve up HTML of links to binaries recently hosted on Google Cloud Storage (a recent addition). We've swapped to using such sites under our control.

jonalmeida commented 2 years ago

We've swapped to using such sites under our control.

@AaronMT swapping to a site under our control is still subject to changes - which is why the tests failed. In the linked pull requests above, you can see that we had to backport one of your patches (https://github.com/mozilla-mobile/fenix/pull/23335) to a previous release branch because the sites under our control had changed.

Another example to consider, is that if you tried to checkout releases_v95 and ran the UI tests there, there would be some tests which are perma-fails now because our test site is no longer the same.

There were complications and concerns with testing local assets is bundling large binaries with the repository (e.g, a large download binary download test), or others such as Enhanced Tracking Protection can not be tested without touching network (as far as I'm aware).

Out of curiosity, would you happen to know how does desktop solve this problem with ETP? I wonder if there is a solution out there that they use which we can also follow.

If these solutions don't work, could we possibly consider versioning our test sites? For example, in #23335, the site changed the page title. Could we create a new URL for that test that ran on a new distinct version? i.e. https://mozilla-mobile.github.io/v2/testapp/. So when the test is updated, we create a new site version that is static and will never change?

diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt
index 11f863248..d4f55278e 100644
--- a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt
+++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt
@@ -1058,7 +1058,7 @@ class SmokeTest {

     @Test
     fun mainMenuInstallPWATest() {
-        val pwaPage = "https://mozilla-mobile.github.io/testapp/"
+        val pwaPage = "https://mozilla-mobile.github.io/v2/testapp/"

         navigationToolbar {
         }.enterURLAndEnterToBrowser(pwaPage.toUri()) {
@@ -1066,7 +1066,7 @@ class SmokeTest {
         }.openThreeDotMenu {
         }.clickInstall {
             clickAddAutomaticallyButton()
-        }.openHomeScreenShortcut("yay app") {
+        }.openHomeScreenShortcut("TEST_APP") {
             mDevice.waitForIdle()
             verifyNavURLBarHidden()
         }
sv-ohorvath commented 1 year ago

I think this problem was solved by starting to use versioning on our internal testapp page. I'll close the issue.