UI Sync Int Tests fix TPS preference #8489

isabelrios · 2020-02-18T17:24:30Z

Sync integration tests were failing due to Bug 1610758 This PR fixes that

Pull Request checklist

Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

After merge

Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

click on Show All Checks,
click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
click on the "Fenix - assemble" task, then click "Run Artifacts".
the APK links should be on the left side of the screen, named for each CPU architecture

codecov-io · 2020-02-18T17:41:38Z

Codecov Report

Merging #8489 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #8489      +/-   ##
============================================
+ Coverage     19.02%   19.06%   +0.03%     
  Complexity      490      490              
============================================
  Files           320      320              
  Lines         12843    12838       -5     
  Branches       1688     1688              
============================================
+ Hits           2443     2447       +4     
+ Misses        10196    10187       -9     
  Partials        204      204

Impacted Files	Coverage Δ	Complexity Δ
...g/mozilla/fenix/addons/AddonsManagementFragment.kt	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...illa/fenix/addons/InstalledAddonDetailsFragment.kt	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...rc/main/java/org/mozilla/fenix/FenixApplication.kt	`13.44% <0%> (+0.22%)`	`3% <0%> (ø)`	⬇️
...mponents/searchengine/FenixSearchEngineProvider.kt	`56.52% <0%> (+4.34%)`	`10% <0%> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 499f38c...fa6998e. Read the comment docs.

isabelrios · 2020-02-24T19:08:14Z

This PR fixes the TPS issue so that we can use it with the new prefs. Also, it adds a condition to run tests only on master as a start. I'm enabling only two tests at the moment to see how they work on the sim on the laptop in MTV. Then, there will be a follow up PR adjusting the timing issues and enabling the rest of tests.

There was a change in fxawebchannel extension and stage sever stop working there, there is a patch to workaround that done by @csadilek (big thanks!!)

rpappalax · 2020-02-24T19:17:52Z

app/src/androidTest/java/org/mozilla/fenix/syncintegration/SyncIntegrationTest.kt

@@ -207,8 +209,7 @@ class SyncIntegrationTest {
    }

    fun historyAfterSyncIsShown() {
-        val historyEntry = mDevice.findObject(By.text("http://www.example.com/"))
-        historyEntry.isEnabled()
+        mDevice.waitNotNull(Until.findObjects(By.text("http://www.example.com/")), TestAssetHelper.waitingTime)


Was wondering if we could use a static asset instead here, but I assume http://www.example.com is an URL used by TPS (and we can't change it), true?

I have to investigate that, on iOS using a static asset did not work because the history was not properly recorded on the iOS device. Also, in this case we are checking on Fenix that the history created on Desktop is shown, so it is kind of static from Fenix side.

I will investigate if creating the history using a static asset on Fenix works, but if possible let's keep that for the follow up PR too

rpappalax · 2020-02-24T20:49:43Z

app/src/androidTest/java/org/mozilla/fenix/syncintegration/test_integration.py

 def test_sync_bookmark_from_desktop(tps, gradlewbuild):
+    os.chdir('app/src/androidTest/java/org/mozilla/fenix/syncintegration/')


this relative path gets repeated over and over. Not sure if it's possible to create a fixture for it instead

I guess that would be doable. Let me improve that in a follow up since now I would prefer check that these tests work on the MTV laptop 🙏

rpappalax · 2020-02-24T21:30:17Z

app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt

+import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.ui.robots.appContext
+
+class AppRequestInterceptor(private val context: Context) : RequestInterceptor {


what is the purpose of this class? 🤔

This is the class that allows us to workaround the issue with fxawebchannel extension not working for stage. Otherwise we can not sync using stage server

I would add a comment to this class as it's not obvious, something like:

/** * Overrides the application's request interceptor to deactivate the FxA web channel * which is not supported on the staging servers. */

isabelrios · 2020-02-27T09:30:45Z

@csadilek sorry for pinging you again, I know you are busy, but just in case this got out of you radar. Just to double check that is safe to land your patch as it is as part of this PR. Thanks again!

csadilek

I would add one comment (see below), and squash the commits before landing, but yes this is safe to land. It has no impact on app code, just makes FxA work in android tests.

csadilek · 2020-02-27T18:30:11Z

app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt

+import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.ui.robots.appContext
+
+class AppRequestInterceptor(private val context: Context) : RequestInterceptor {


I would add a comment to this class as it's not obvious, something like:

/** * Overrides the application's request interceptor to deactivate the FxA web channel * which is not supported on the staging servers. */

Sync Int Tests fix TPS preference add webext navigation enable only two tests add comment as per reviewer suggestion

…ile#8489)

isabelrios force-pushed the ui-sync-int-tests-fix-TPS branch 2 times, most recently from 1009537 to 20e3da8 Compare February 24, 2020 16:41

isabelrios changed the title ~~(WIP) UI Sync Int Tests fix TPS preference~~ UI Sync Int Tests fix TPS preference Feb 24, 2020

isabelrios requested review from rpappalax and csadilek February 24, 2020 18:56

isabelrios marked this pull request as ready for review February 24, 2020 18:57

rpappalax approved these changes Feb 25, 2020

View reviewed changes

isabelrios force-pushed the ui-sync-int-tests-fix-TPS branch from bf5e08d to 166720a Compare February 26, 2020 12:57

isabelrios force-pushed the ui-sync-int-tests-fix-TPS branch from 166720a to d09bd20 Compare February 27, 2020 11:34

csadilek approved these changes Feb 27, 2020

View reviewed changes

i# This is a combination of 5 commits.

fa6998e

Sync Int Tests fix TPS preference add webext navigation enable only two tests add comment as per reviewer suggestion

isabelrios force-pushed the ui-sync-int-tests-fix-TPS branch from d09bd20 to fa6998e Compare February 27, 2020 18:42

AaronMT merged commit b432f5c into mozilla-mobile:master Feb 27, 2020

gmierz pushed a commit to gmierz/fenix that referenced this pull request Mar 5, 2020

Update UI Sync integration tests and fix TPS preference. (mozilla-mob…

718b0d8

…ile#8489)

liuche mentioned this pull request Mar 12, 2020

Releng 4.1 #9116

Closed

32 tasks

liuche mentioned this pull request Mar 24, 2020

Releng for 4.2 #9343

Closed

32 tasks

Mugurell mentioned this pull request Jan 17, 2022

[UI smoke test] Open links in apps UI test #22623

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI Sync Int Tests fix TPS preference #8489

UI Sync Int Tests fix TPS preference #8489

isabelrios commented Feb 18, 2020

codecov-io commented Feb 18, 2020 •

edited

isabelrios commented Feb 24, 2020

rpappalax Feb 24, 2020

isabelrios Feb 25, 2020

rpappalax Feb 24, 2020

isabelrios Feb 25, 2020

rpappalax Feb 24, 2020

isabelrios Feb 25, 2020

csadilek Feb 27, 2020

isabelrios commented Feb 27, 2020

csadilek left a comment

csadilek Feb 27, 2020

		def test_sync_bookmark_from_desktop(tps, gradlewbuild):
		os.chdir('app/src/androidTest/java/org/mozilla/fenix/syncintegration/')

UI Sync Int Tests fix TPS preference #8489

UI Sync Int Tests fix TPS preference #8489

Conversation

isabelrios commented Feb 18, 2020

Pull Request checklist

After merge

To download an APK when reviewing a PR:

codecov-io commented Feb 18, 2020 • edited

Codecov Report

isabelrios commented Feb 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios commented Feb 27, 2020

csadilek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Feb 18, 2020 •

edited