Closes #7176: Add to the configuration file for instrumentation tests components that use Room #7196

Amejia481 · 2020-06-01T02:26:55Z

Closes #7176

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry or does not need one
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

components that use Room

Amejia481 · 2020-06-01T02:28:47Z

taskcluster/ci/test/kind.yml

+            post-gradlew:
+                - ['automation/taskcluster/androidTest/ui-test.sh', 'feature-sitepermissions', 'arm', '1']
+        treeherder:
+            symbol: 'unit-sitepermissions'


If I use a symbol that have more that 20 character I get an error.

Amejia481 · 2020-06-01T02:31:49Z

automation/taskcluster/androidTest/ui-test.sh

 if [[ "${component}" != samples-* ]]
 then
    # Case 1: tests for any component (but NOT samples, NOT real UI tests)
-    APK_APP="./samples/${component}/build/outputs/apk/geckoNightly/debug/samples-${component}-geckoNightly-debug.apk"


I'm not sure why we need to pass these to parameter to FLANK when we just want to run instrumentation test no UI test.

Was not sure there could be a different sample that could run similar tests. If there is not any and it is not going to be, then, no need to parameter that

Thanks 👍

It looks like Flank needs the extra parameter without it the compilation fails. I will left the fixed string as it is.

I tried to excluded it and I got this result

/usr/bin/java -jar /builds/worker/test-tools/flank.jar android run --config=./automation/taskcluster/androidTest/flank-arm.yml --max-test-shards=1 --test=./components/feature/pwa/build/outputs/apk/androidTest/debug/feature-pwa-debug-androidTest.apk --project=moz-android-components-230120 exitcode=0 version: v20.05.2 revision: 7618ebdb5a74db4b448de0b93357221de3ab10c2 Unmatched argument at index 6: 'exitcode=0' Run tests on Firebase Test Lab

codecov · 2020-06-01T02:31:50Z

Codecov Report

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

@@            Coverage Diff             @@
##             master    #7196    +/-   ##
==========================================
  Coverage     77.24%   77.24%            
+ Complexity     5045     5032    -13     
==========================================
  Files           674      673     -1     
  Lines         24703    24580   -123     
  Branches       3645     3639     -6     
==========================================
- Hits          19082    18987    -95     
+ Misses         4113     4088    -25     
+ Partials       1508     1505     -3

Impacted Files	Coverage Δ	Complexity Δ
...ponents/support/migration/SearchEngineMigration.kt	`100.00% <0.00%> (ø)`	`6.00% <0.00%> (ø%)`
...onents/support/sync/telemetry/BaseGleanSyncPing.kt	`100.00% <0.00%> (ø)`	`11.00% <0.00%> (ø%)`
...ava/mozilla/components/ui/tabcounter/TabCounter.kt
...components/support/sync/telemetry/SyncTelemetry.kt	`87.28% <0.00%> (+1.57%)`	`41.00% <0.00%> (ø%)`

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 2ac91e3...b7c015a. Read the comment docs.

Amejia481 · 2020-06-01T02:34:18Z

taskcluster/ci/test/kind.yml

                - ['automation/taskcluster/androidTest/ui-test.sh', 'samples-glean', 'arm', '1']
        treeherder:
            symbol: 'ui-samples-glean'
+    android-feature-pwa:


@pocmo you can see all the new test running on this branch https://github.com/mozilla-mobile/android-components/commits/android_test on this commit 945ffee

Amejia481 · 2020-06-01T02:37:01Z

automation/taskcluster/androidTest/ui-test.sh

-    APK_APP="./samples/${component}/build/outputs/apk/geckoNightly/debug/samples-${component}-geckoNightly-debug.apk"
-    APK_TEST="./components/${component}/engine-gecko-nightly/build/outputs/apk/androidTest/debug/browser-engine-gecko-nightly-debug-androidTest.apk"
+    APK_APP="./samples/browser/build/outputs/apk/geckoNightly/debug/samples-browser-geckoNightly-debug.apk"
+    if [[ "${component}" == *"-"* ]]


If I don't find component like feature-*, concept-*, browser-* ... etc I fallback to the previous implementation

gabrielluong · 2020-06-01T06:18:05Z

...re/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/TopSiteStorageTest.kt

                "INSERT INTO " +
                    "top_sites " +
-                    "(title, url, isDefault, created_at) " +
+                    "(title, url, is_default, created_at) " +


How come we are fixing the migration from v1 to v2? We knew we misused isDefault instead of is_default in v2, but we kinda agreed to not change it and just fix it in the migrate_2_3.

We encountered that problem because, we didn't realize we were not running these tests on CI. Now I'm activating the tests on CI and Room is complaining (throwing the same error that we fixed on migration 3), if I don't fix the issue we are not going to be able to run the test on CI.

If you run the test TopSiteStorageTest.kt#migrate1to2 locally, you will see the below error

java.lang.IllegalStateException: Migration didn't properly handle: top_sites Expected: TableInfo{name='top_sites', columns={created_at=Column{name='created_at', type='INTEGER', affinity='3', notNull=true, primaryKeyPosition=0, defaultValue='null'}, id=Column{name='id', type='INTEGER', affinity='3', notNull=false, primaryKeyPosition=1, defaultValue='null'}, title=Column{name='title', type='TEXT', affinity='2', notNull=true, primaryKeyPosition=0, defaultValue='null'}, is_default=Column{name='is_default', type='INTEGER', affinity='3', notNull=true, primaryKeyPosition=0, defaultValue='null'}, url=Column{name='url', type='TEXT', affinity='2', notNull=true, primaryKeyPosition=0, defaultValue='null'}}, foreignKeys=[], indices=[]} found: TableInfo{name='top_sites', columns={isDefault=Column{name='isDefault', type='INTEGER', affinity='3', notNull=true, primaryKeyPosition=0, defaultValue='0'}, created_at=Column{name='created_at', type='INTEGER', affinity='3', notNull=true, primaryKeyPosition=0, defaultValue='null'}, id=Column{name='id', type='INTEGER', affinity='3', notNull=false, primaryKeyPosition=1, defaultValue='null'}, title=Column{name='title', type='TEXT', affinity='2', notNull=true, primaryKeyPosition=0, defaultValue='null'}, url=Column{name='url', type='TEXT', affinity='2', notNull=true, primaryKeyPosition=0, defaultValue='null'}}, foreignKeys=[], indices=[]} at androidx.room.RoomOpenHelper.onUpgrade(RoomOpenHelper.java:103) at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.onUpgrade(FrameworkSQLiteOpenHelper.java:124) at android.database.sqlite.SQLiteOpenHelper.getDatabaseLocked(SQLiteOpenHelper.java:417) at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:317) at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getWritableSupportDatabase(FrameworkSQLiteOpenHelper.java:92) at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper.getWritableDatabase(FrameworkSQLiteOpenHelper.java:53) at androidx.room.testing.MigrationTestHelper.openDatabase(MigrationTestHelper.java:246) at androidx.room.testing.MigrationTestHelper.runMigrationsAndValidate(MigrationTestHelper.java:236) at mozilla.components.feature.top.sites.TopSiteStorageTest.migrate1to2(TopSiteStorageTest.kt:141) at java.lang.reflect.Method.invoke(Native Method) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47) 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 org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55) at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55) at org.junit.rules.RunRules.evaluate(RunRules.java:20) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268) at org.junit.runners.ParentRunner.run(ParentRunner.java:363) at org.junit.runners.Suite.runChild(Suite.java:128) at org.junit.runners.Suite.runChild(Suite.java:27) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268) at org.junit.runners.ParentRunner.run(ParentRunner.java:363) at org.junit.runner.JUnitCore.run(JUnitCore.java:137) at org.junit.runner.JUnitCore.run(JUnitCore.java:115) at androidx.test.internal.runner.TestExecutor.execute(TestExecutor.java:56) at androidx.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:392) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2196)

That means this could have happened to users too (if we would have shipped all versions)?

Yes, this happened to users. We didn't caught it because we didn't run the tests on CI. Lucky now we have migration 3 that is fixing the issue for all users.

isabelrios · 2020-06-02T16:21:41Z

bors try

bors · 2020-06-02T16:34:26Z

try

Build succeeded:

complete-push

isabelrios

LGTM, after double checking that all tests are passing :)

pocmo

👍

Amejia481 · 2020-06-03T14:46:46Z

bors r=gabrielluong,pocmo,isabelrios

bors · 2020-06-03T15:10:18Z

Build succeeded:

complete-push

Amejia481 added 2 commits May 31, 2020 22:16

Fix top sites instrumentation tests

726daec

Add to the configuration file for instrumentation tests

b7c015a

components that use Room

Amejia481 added the 🕵️‍♀️ needs review PRs that need to be reviewed label Jun 1, 2020

Amejia481 requested review from isabelrios and pocmo June 1, 2020 02:26

Amejia481 assigned pocmo and isabelrios Jun 1, 2020

Amejia481 commented Jun 1, 2020

View reviewed changes

Amejia481 changed the title ~~Add to the configuration file for instrumentation tests components that use Room~~ Closes #7176: Add to the configuration file for instrumentation tests components that use Room Jun 1, 2020

gabrielluong reviewed Jun 1, 2020

View reviewed changes

bors bot pushed a commit that referenced this pull request Jun 2, 2020

Try #7196:

5e74801

isabelrios approved these changes Jun 2, 2020

View reviewed changes

pocmo approved these changes Jun 3, 2020

View reviewed changes

pocmo mentioned this pull request Jun 3, 2020

Issue #7194: Create a Browser Containers component #7195

Merged

4 tasks

Amejia481 added 🛬 needs landing PRs that are ready to land and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels Jun 3, 2020

bors bot merged commit 8998e8c into mozilla-mobile:master Jun 3, 2020

Closes #7176: Add to the configuration file for instrumentation tests components that use Room #7196

Closes #7176: Add to the configuration file for instrumentation tests components that use Room #7196

Uh oh!

Conversation

Amejia481 commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Closes #7176

Pull Request checklist

After merge

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amejia481 Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isabelrios commented Jun 2, 2020

Uh oh!

bors bot commented Jun 2, 2020

try

Uh oh!

isabelrios left a comment

Choose a reason for hiding this comment

Uh oh!

pocmo left a comment

Choose a reason for hiding this comment

Uh oh!

Amejia481 commented Jun 3, 2020

Uh oh!

bors bot commented Jun 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Amejia481 commented Jun 1, 2020 •

edited

Loading

Amejia481 Jun 3, 2020 •

edited

Loading

codecov bot commented Jun 1, 2020 •

edited

Loading