Skip to content

Commit

Permalink
[RunAllTests] Fix #2941: Fix glide image loading in Bazel (#2979)
Browse files Browse the repository at this point in the history
* Add support for loading proto versions of lessons.

* Upgrade Espresso versions to work around build issues.

Note that I can't actually verify Espresso tests are working with these
versions since the Bazel project doesn't yet have Espresso tests set up.

* Lint fix post-copying from #2927.

* Fix Glide in Bazel.

* Remove image loading annotation.

This will be added in a later branch in the chain.

* Add caching module to needed test suites.

* Add support for importing text protos as assets.

This actually provides Bazel-only support for converting text protos to
binary and including those as assets so that we can avoid checking in
binary protos to the codebase.

* Add gitignore for Android Studio Bazel plugin output.

* Import textproto versions of existing dev assets.

This imports local conversions of all existing dev assets as text proto
leveraging the Bazel-only conversion system to convert these to binary
protos and include them as assets.

This is a necessary step to both eventually replace JSON assets with
text proto, and to test that the new proto loading pipeline is working
as designed.

* Undo Espresso version change since it breaks Bazel tests.

* Fix tests.

Various attempts to fix existing tests + move some of the domain tests
over to actually using proto when running with Bazel.

App module tests are hanging currently for unknown reasons (only in
Bazel).

* Use correct Espresso core.

3.1.0 causes tests to hang indefinitely when run with Bazel.

* Move coroutine dispatchers.

This moves the coroutine dispatchers to a new threading subpackage. It
also simplifies the TestCoroutineDispatcher interface & implementations
(which required moving runUntilIdle to CoroutineExecutorServiceTest).

The test coroutine annotations were also moved to their own files.

* Refactor testing utilities build graph.

This introduces new Bazel libraries for:
- Test dispatchers
- Fake system/Oppia clocks
- Robolectric dependencies

* Introduce dedicated test for TestCoroutineDispatcher.

This also fixes some threading issues in the dispatcher, and clarifies
some of its API that was previously unclear (and can lead to subtle race
conditions).

* Add tests for TestCoroutineDispatcher.

This required a bunch of refactoring, adding the first examples of
dedicated Bazel test build files, and introducing a new test pattern for
sharing code between different implementations. Further, it refined the
API for TesCoroutineDispatcher & fixed some issues in the
implementations (especially Robolectric).

Tests verified as non-flaky in Robolectric across 1000 runs, and 0.2%
flaky across 1000 runs for Espresso (though that may be fixed; waiting
on a follow-up run to confirm stability).

* Workaround ktlint semicolon issue.

* Lint fixes.

* Fix test post-merge.

* Move CoroutineDispatcher to be part of threading.

* Lint fixes.

* Undo version upgrade.

* Fix broken tests.

* Lint fixes.

* Add a bit more proto loading test coverage.

* Comment fix.

* Lint fixes.

* Address earlier TODO.

* Post-merge restructure.

* Post-merge fixes.

This removes unneeded dependencies from PersistentCacheStore, including
Glide.

* Fix broken test build.

* Post-merge lint fixes.
  • Loading branch information
BenHenning authored Apr 23, 2021
1 parent adae599 commit 11fe81e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ kt_android_library(
deps = [
":dagger",
"//model:profile_java_proto_lite",
"//third_party:androidx_appcompat_appcompat",
"//third_party:androidx_lifecycle_lifecycle-livedata-ktx",
"//third_party:com_caverock_androidsvg-aar",
"//third_party:com_github_bumptech_glide_glide",
"//utility",
"//utility/src/main/java/org/oppia/android/util/profile:directory_management_util",
],
Expand Down
12 changes: 12 additions & 0 deletions third_party/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,15 @@ java_library(
"//third_party:com_squareup_moshi_moshi-kotlin-codegen",
],
)

# Define a separate target for the Glide annotation processor compiler. Unfortunately, this library
# can't encapsulate all of Glide (i.e. by exporting the main Glide dependency) since that includes
# Android assets which java_library targets do not export.
java_library(
name = "glide_compiler",
exported_plugins = ["//tools:glide_compiler_plugin"],
visibility = ["//visibility:public"],
exports = [
"//third_party:com_github_bumptech_glide_compiler",
],
)
1 change: 1 addition & 0 deletions third_party/versions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ MAVEN_PRODUCTION_DEPENDENCY_VERSIONS = {
"com.caverock:androidsvg-aar": "1.4",
"com.chaos.view:pinview": "1.4.4",
"com.crashlytics.sdk.android:crashlytics": "2.9.8",
"com.github.bumptech.glide:compiler": "4.11.0",
"com.github.bumptech.glide:glide": "4.11.0",
"com.google.android.material:material": "1.2.0-alpha02",
"com.google.android:flexbox": "2.0.1",
Expand Down
8 changes: 8 additions & 0 deletions tools/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,11 @@ java_plugin(
"//third_party:com_squareup_moshi_moshi-kotlin-codegen",
],
)

java_plugin(
name = "glide_compiler_plugin",
generates_api = True,
processor_class = "com.bumptech.glide.annotation.compiler.GlideAnnotationProcessor",
visibility = ["//third_party:__pkg__"],
deps = ["//third_party:com_github_bumptech_glide_compiler"],
)
1 change: 1 addition & 0 deletions utility/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ kt_android_library(
"//third_party:com_caverock_androidsvg-aar",
"//third_party:com_github_bumptech_glide_glide",
"//third_party:com_google_guava_guava",
"//third_party:glide_compiler",
"//third_party:org_jetbrains_kotlinx_kotlinx-coroutines-core",
"//utility/src/main/java/org/oppia/android/util/datetime:date_time_util",
"//utility/src/main/java/org/oppia/android/util/gcsresource:annotations",
Expand Down

0 comments on commit 11fe81e

Please sign in to comment.