Fix all tests in EnvironmentsTest #1358
Conversation
There was a problem hiding this comment.
Thanks for the patch!
We haven't really paid much attention to tests but we should. WRT the glean version change, it's weird that v56 is not compatible while v55 is, is it because some removed symbol?
Apart from that, I think it'd be nice to add a test phase to the CI, if tests are not automatically run they are not very useful. That said access to github CI is limitted for non-organization members so I'd say just give it a try, if you don't manage to get it right we can do it in a followup. Thanks!
Hmm, I really don't know much about the why, it seems they broke backward compatibility between v55 and v56. I wonder if there is a better way to access the
Yeah, of course, I'll give it a try. |
HoussemNasri
force-pushed
the
fix-environments-tests
branch
3 times, most recently
from
f4efb09
to
9870416
Compare
|
Hi @svillar, I added a new workflow to run unit tests. It runs all unit tests in the app module using the debug and then the release build . Also, I made another change: I separated the "deployment" step, which uploads the built APK, into its own job. This way, if the tests don't pass, the upload won't happen. |
HoussemNasri
force-pushed
the
fix-environments-tests
branch
2 times, most recently
from
9754c45
to
5754385
Compare
|
I'm still getting this error |
|
@HoussemNasri OK I don't want you to get blocked on this, and the fixes are still good, so we can figure out the CI later (as it's difficult for you as you don't have full access to CI as we do). Mind removing the changes related to CI? I'll approve and merge after that. |
- Because it's compatible with android_components 121
HoussemNasri
force-pushed
the
fix-environments-tests
branch
from
5754385
to
5396de1
Compare
Ah this is happening without the CI, I thought it was caused by the CI patch. We need to fix it then |
|
Could it be because the source branch is on my fork, not on upstream? I saw no similar errors in the pull requests that you and the other maintainers opened. |
There was a problem hiding this comment.
Sorry for the back and forth. I'd really love to land this once for all but I've tested it locally and although the tests do pass I get tons of backtraces with the following backtrace:
System.logW: A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
java.lang.Throwable: Explicit termination method 'close' not called
at dalvik.system.CloseGuard.$$robo$$dalvik_system_CloseGuard$open(CloseGuard.java:221)
at jdk.internal.reflect.GeneratedMethodAccessor5.invoke(Unknown Source)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at org.robolectric.shadows.ShadowCloseGuard$CloseGuardReflector$$Reflector15.open(Unknown Source)
at org.robolectric.shadows.ShadowCloseGuard.open(ShadowCloseGuard.java:38)
at dalvik.system.CloseGuard.open(CloseGuard.java)
at android.database.sqlite.SQLiteDatabase.$$robo$$android_database_sqlite_SQLiteDatabase$openInner(SQLiteDatabase.java:881)
at android.database.sqlite.SQLiteDatabase.openInner(SQLiteDatabase.java)
at android.database.sqlite.SQLiteDatabase.$$robo$$android_database_sqlite_SQLiteDatabase$open(SQLiteDatabase.java:865)
at android.database.sqlite.SQLiteDatabase.open(SQLiteDatabase.java)
at android.database.sqlite.SQLiteDatabase.$$robo$$android_database_sqlite_SQLiteDatabase$openDatabase(SQLiteDatabase.java:739)
at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java)
at android.database.sqlite.SQLiteDatabase.$$robo$$android_database_sqlite_SQLiteDatabase$createInMemory(SQLiteDatabase.java:920)
at android.database.sqlite.SQLiteDatabase.createInMemory(SQLiteDatabase.java)
at android.database.sqlite.SQLiteOpenHelper.$$robo$$android_database_sqlite_SQLiteOpenHelper$getDatabaseLocked(SQLiteOpenHelper.java:350)
at android.database.sqlite.SQLiteOpenHelper.getDatabaseLocked(SQLiteOpenHelper.java)
at android.database.sqlite.SQLiteOpenHelper.$$robo$$android_database_sqlite_SQLiteOpenHelper$getWritableDatabase(SQLiteOpenHelper.java:298)
at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java)
at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getWritableOrReadableDatabase(FrameworkSQLiteOpenHelper.kt:232)
at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.innerGetDatabase(FrameworkSQLiteOpenHelper.kt:177)
at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getSupportDatabase(FrameworkSQLiteOpenHelper.kt:151)
at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper.getWritableDatabase(FrameworkSQLiteOpenHelper.kt:104)
at androidx.room.RoomDatabase.inTransaction(RoomDatabase.kt:638)
at androidx.room.RoomDatabase.assertNotSuspendingTransaction(RoomDatabase.kt:457)
at androidx.work.impl.model.SystemIdInfoDao_Impl.getWorkSpecIds(SystemIdInfoDao_Impl.java:163)
at androidx.work.impl.background.systemjob.SystemJobScheduler.reconcileJobs(SystemJobScheduler.java:311)
at androidx.work.impl.utils.ForceStopRunnable.cleanUp(ForceStopRunnable.java:278)
at androidx.work.impl.utils.ForceStopRunnable.forceStopRunnable(ForceStopRunnable.java:242)
at androidx.work.impl.utils.ForceStopRunnable.run(ForceStopRunnable.java:134)
at androidx.work.impl.utils.SerialExecutorImpl$Task.run(SerialExecutorImpl.java:96)
at androidx.work.testing.SynchronousExecutor.execute(SynchronousExecutor.java:30)
at androidx.work.impl.utils.SerialExecutorImpl.scheduleNext(SerialExecutorImpl.java:60)
at androidx.work.impl.utils.SerialExecutorImpl.execute(SerialExecutorImpl.java:51)
at androidx.work.impl.utils.taskexecutor.TaskExecutor.executeOnTaskThread(TaskExecutor.java:44)
at androidx.work.impl.WorkManagerImpl.internalInit(WorkManagerImpl.java:837)
at androidx.work.impl.WorkManagerImpl.<init>(WorkManagerImpl.java:293)
at androidx.work.impl.WorkManagerImpl.<init>(WorkManagerImpl.java:256)
at androidx.work.testing.TestWorkManagerImpl.<init>(TestWorkManagerImpl.java:63)
at androidx.work.testing.WorkManagerTestInitHelper.initializeTestWorkManager(WorkManagerTestInitHelper.java:70)
at androidx.work.testing.WorkManagerTestInitHelper.initializeTestWorkManager(WorkManagerTestInitHelper.java:44)
at mozilla.telemetry.glean.testing.GleanTestRule.starting(GleanTestRule.kt:49)
at org.junit.rules.TestWatcher.startingQuietly(TestWatcher.java:113)
at org.junit.rules.TestWatcher.access$000(TestWatcher.java:52)
at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:59)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:588)
at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:290)
at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:101)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:840)
I guess it's because the migration to mozilla glean. We need to fix that before landing.
|
Ah and BTW let's re-add the patch to run tests in CI since the R8 error is not related to that. |
Hmmm, I reset hard and force-pushed so it's gone, but it would have needed more work anyway, better add it in another PR. |
Summary
The
glean-native-forUnitTestsandglean-gradle-pluginversions were updated to55.0.0to address an issue where running tests resulted in ajava.lang.UnsatisfiedLinkError: Error looking up function 'ffi_glean_uniffi_contract_version'exception. Following this, I conducted adjustments to the JSON and unit tests through trial and error until they all passed successfully.I also went through the deleted changes in this commit 0a9ca17 which I think what made the tests fail to understand the expected behavior of the tests.
The choice of version 55.0.0 was based on its compatibility with Mozilla android components v121. For context: https://github.com/mozilla-mobile/firefox-android/blob/979fbe8d7fe04a9b09fe657bb787fda6f4d5ab42/android-components/plugins/dependencies/src/main/java/DependenciesPlugin.kt#L46