Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android MVVM Module + Android Player test package restructure #307

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions android/demo/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ maven_test = [
maven = maven_main + maven_test

main_deps = parse_coordinates(maven_main) + [
"//android/player-ui:player-mvvm",
"//jvm/utils",
"//plugins/reference-assets/android:assets",
"//plugins/common-types/jvm:common-types",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package com.intuit.playerui.android.reference.demo.lifecycle

import com.intuit.playerui.android.AndroidPlayer
import com.intuit.playerui.android.AndroidPlayer.Config
import com.intuit.playerui.android.lifecycle.PlayerViewModel
import com.intuit.playerui.android.mvvm.lifecycle.PlayerViewModel
import com.intuit.playerui.android.reference.assets.ReferenceAssetsPlugin
import com.intuit.playerui.core.managed.AsyncFlowIterator
import com.intuit.playerui.core.player.state.PlayerFlowState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import androidx.navigation.fragment.findNavController
import com.afollestad.materialdialogs.MaterialDialog
import com.alexii.j2v8debugger.StethoHelper
import com.intuit.playerui.android.lifecycle.ManagedPlayerState
import com.intuit.playerui.android.lifecycle.PlayerViewModel
import com.intuit.playerui.android.mvvm.lifecycle.PlayerViewModel
import com.intuit.playerui.android.mvvm.ui.PlayerFragment
import com.intuit.playerui.android.reference.demo.lifecycle.DemoPlayerViewModel
import com.intuit.playerui.android.ui.PlayerFragment
import com.intuit.playerui.core.bridge.serialization.json.prettify
import com.intuit.playerui.core.bridge.toJson
import com.intuit.playerui.core.managed.AsyncFlowIterator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import androidx.navigation.ui.onNavDestinationSelected
import androidx.navigation.ui.setupActionBarWithNavController
import androidx.navigation.ui.setupWithNavController
import com.google.android.material.navigation.NavigationView
import com.intuit.playerui.android.mvvm.ui.PlayerFragment
import com.intuit.playerui.android.reference.demo.R
import com.intuit.playerui.android.reference.demo.model.AssetMock
import com.intuit.playerui.android.reference.demo.model.StringMock
import com.intuit.playerui.android.ui.PlayerFragment
import com.intuit.playerui.utils.mocks.ClassLoaderMock
import com.intuit.playerui.utils.mocks.Mock
import com.intuit.playerui.utils.mocks.getFlow
Expand Down
3 changes: 2 additions & 1 deletion android/deps.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
load("//jvm/dependencies:versions.bzl", "versions")
load("//android/player:deps.bzl", player = "maven")
load("//android/demo:deps.bzl", demo = "maven")
load("//android/player-ui:deps.bzl", ui = "maven")

android = [
# Grab Databinding
Expand All @@ -18,4 +19,4 @@ android = [
"androidx.fragment:fragment-ktx:%s" % versions.androidx.fragment,
]

maven = android + player + demo
maven = android + player + demo + ui
46 changes: 46 additions & 0 deletions android/player-ui/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
load(":deps.bzl", "main_deps", "test_deps")
load("@build_constants//:constants.bzl", "VERSION")
load("//jvm:build.bzl", "distribution")
load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")
load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
load("@junit//junit5-jupiter-starter-bazel:junit5.bzl", "kt_jvm_junit5_test")
load("@rules_player//kotlin:lint.bzl", "lint")
load("@grab_bazel_common//tools/databinding:databinding.bzl", "kt_db_android_library")

kt_db_android_library(
name = "player-mvvm",
srcs = glob(["src/main/java/**/*.kt"]),
custom_package = "com.intuit.playerui.android.mvvm",
manifest = ":src/main/AndroidManifest.xml",
resource_files = glob(["src/main/res/**"]),
tags = ["maven_coordinates=com.intuit.playerui.android:player-mvvm:aar:%s" % VERSION],
visibility = ["//visibility:public"],
deps = main_deps,
)

distribution(
name = "player-mvvm",
lib_name = "player-mvvm-databinding",
maven_coordinates = "com.intuit.playerui.android:player-mvvm:%s" % VERSION,
)

kt_jvm_junit5_test(
name = "player-mvvm-tests",
srcs = glob(["src/test/java/**/*.kt"]),
kotlinc_opts = "//jvm:test_options",
associates = ["//android/player:player-associates"],
test_package = "com.intuit.playerui.android.mvvm",
deps = [
"//android/player:player",
":player-mvvm",
"//android/player/src/test/java/android/util:android_utils_stub",
"//android/player/src/test/java/com/intuit/playerui/android/utils:android_test_utils",
"//android/player/src/test/java/com/intuit/playerui/android/utils:android_log_util"
] + test_deps,
)

lint(
name = "player-mvvm",
srcs = glob(["src/**/*.kt"]),
lint_config = "//jvm:lint_config",
)
34 changes: 34 additions & 0 deletions android/player-ui/deps.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
load("//jvm/dependencies:versions.bzl", "versions")
load("@rules_player//maven:parse_coordinates.bzl", "parse_coordinates")

maven = [
# UI helpers
"androidx.core:core-ktx:%s" % versions.androidx.core,
"androidx.appcompat:appcompat:%s" % versions.androidx.appcompat,
"androidx.transition:transition:%s" % versions.androidx.transition,

# Lifecycle
"androidx.lifecycle:lifecycle-runtime-ktx:%s" % versions.androidx.lifecycle,
"androidx.lifecycle:lifecycle-viewmodel-ktx:%s" % versions.androidx.lifecycle,

# Default fallback
"androidx.constraintlayout:constraintlayout:%s" % versions.androidx.constraintlayout,
]

main_deps = parse_coordinates(maven) + [
# JVM plugin deps
"//android/player:player",
"//plugins/coroutines/jvm:coroutines",
]

main_resources = [
# TS core deps
"//plugins/partial-match-fingerprint/core:PartialMatchFingerprintPlugin_Bundles",
"//core/partial-match-registry:Registry_Bundles",
]

test_deps = [
"@grab_bazel_common//tools/test:mockable-android-jar",
"@maven//:io_mockk_mockk",
"@maven//:org_jetbrains_kotlinx_kotlinx_coroutines_test",
]
3 changes: 3 additions & 0 deletions android/player-ui/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.intuit.player.android.mvvm">
<uses-sdk android:minSdkVersion="14" />
</manifest>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.intuit.playerui.android.lifecycle
package com.intuit.playerui.android.mvvm.lifecycle

import android.app.Application
import androidx.lifecycle.ViewModel
Expand All @@ -7,6 +7,7 @@ import androidx.lifecycle.viewModelScope
import com.intuit.playerui.android.AndroidPlayer
import com.intuit.playerui.android.AndroidPlayerPlugin
import com.intuit.playerui.android.asset.RenderableAsset
import com.intuit.playerui.android.lifecycle.ManagedPlayerState
import com.intuit.playerui.core.bridge.runtime.Runtime
import com.intuit.playerui.core.experimental.ExperimentalPlayerApi
import com.intuit.playerui.core.managed.AsyncFlowIterator
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.intuit.playerui.android.ui
package com.intuit.playerui.android.mvvm.ui

import android.content.Context
import android.os.Bundle
Expand All @@ -14,14 +14,14 @@ import androidx.transition.Transition
import com.intuit.playerui.android.AndroidPlayer
import com.intuit.playerui.android.asset.RenderableAsset
import com.intuit.playerui.android.asset.SuspendableAsset
import com.intuit.playerui.android.databinding.DefaultFallbackBinding
import com.intuit.playerui.android.databinding.FragmentPlayerBinding
import com.intuit.playerui.android.extensions.into
import com.intuit.playerui.android.extensions.intoOnMain
import com.intuit.playerui.android.extensions.transitionInto
import com.intuit.playerui.android.lifecycle.ManagedPlayerState
import com.intuit.playerui.android.lifecycle.PlayerViewModel
import com.intuit.playerui.android.lifecycle.fail
import com.intuit.playerui.android.mvvm.databinding.DefaultFallbackBinding
import com.intuit.playerui.android.mvvm.databinding.FragmentPlayerBinding
import com.intuit.playerui.android.mvvm.lifecycle.PlayerViewModel
import com.intuit.playerui.android.mvvm.lifecycle.fail
import com.intuit.playerui.core.experimental.ExperimentalPlayerApi
import com.intuit.playerui.core.managed.AsyncFlowIterator
import kotlinx.coroutines.CoroutineScope
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.intuit.playerui.android.lifecycle
package com.intuit.playerui.android.mvvm.lifecycle

import android.util.Level
import android.util.clearLogs
import com.intuit.playerui.android.lifecycle.ManagedPlayerState
import com.intuit.playerui.android.utils.SimpleAsset
import com.intuit.playerui.android.utils.clearLogs
import com.intuit.playerui.core.bridge.PlayerRuntimeException
import com.intuit.playerui.core.bridge.runtime.Runtime
import com.intuit.playerui.core.managed.AsyncFlowIterator
Expand Down
27 changes: 20 additions & 7 deletions android/player/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ load("@build_constants//:constants.bzl", "VERSION")
load("//jvm:build.bzl", "distribution")
load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")
load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
load("@junit//junit5-jupiter-starter-bazel:junit5.bzl", "kt_jvm_junit5_test")
load("//android/player:build.bzl", "kt_player_test")
load("@rules_player//kotlin:lint.bzl", "lint")
load("@grab_bazel_common//tools/databinding:databinding.bzl", "kt_db_android_library")

Expand All @@ -26,13 +26,26 @@ distribution(
maven_coordinates = "com.intuit.playerui:android:%s" % VERSION,
)

kt_jvm_junit5_test(
kt_player_test(
name = "player-tests",
srcs = glob(["src/test/java/**"]),
associates = [":player-kotlin"],
kotlinc_opts = "//jvm:test_options",
test_package = "com.intuit.playerui.android",
deps = [":player"] + test_deps,
srcs = [
"src/test/java/com/intuit/playerui/android/AndroidPlayerTest.kt",
"src/test/java/com/intuit/playerui/android/AssetContextTest.kt",
"src/test/java/com/intuit/playerui/android/BrokenAssetTest.kt"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first two are not great because the test target should really be in the test package, but it's moved here because of the associates line below, allowing the tests to access internal player API.

the BrokenAssetTest is really bad since i had to move it out of the original package, otherwise i couldn't reference it as a source. It's really the same problem trying to access internal API

the main problem here is :player-kotlin target is created as an intermediate target by the db android macro, and the visibility isn't marked as public so it can only be used from this package. The end result target does not meet the requirements to be used as an associate

I could either modify the grab rules or we need to write some hack, maybe wrapper library to expose :player-kotlin? Is there a better way

]
)

filegroup(
name = "manifest",
srcs = ["src/main/AndroidManifest.xml"],
visibility = ["//visibility:public"]
)

kt_jvm_library(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a terrible workaround for internals in Constants.kt. Since the vals are internal but they're used for the test utils in utils/BUILD, we make this target to use as a publicly visible associate

Copy link
Contributor Author

@brocollie08 brocollie08 Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this commit this actually encapsulates everything, so then i can move BrokenAssetTest.kt file back to where it belongs and get rid of this test target above and place it into the test package. I just don't know if this is the way we want to go

name = "player-associates",
srcs = glob(["src/main/java/**/*.kt"]) + [":player-stubs_binding.srcjar"],
deps = main_deps + ["@grab_bazel_common//tools/android:android_sdk", ":player-r-classes"],
visibility = ["//visibility:public"]
)

lint(
Expand Down
27 changes: 27 additions & 0 deletions android/player/build.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
load("@junit//junit5-jupiter-starter-bazel:junit5.bzl", "kt_jvm_junit5_test")
load("@rules_player//kotlin:lint.bzl", "lint")
load("//android/player:deps.bzl", "test_deps")

def kt_player_test(
name,
srcs = [],
associates = [],
deps = []):
kt_jvm_junit5_test(
name = name,
srcs = srcs,
associates = ["//android/player:player-associates"],
kotlinc_opts = "//jvm:test_options",
test_package = "com.intuit.playerui.android",
deps = deps + [
"//android/player:player",
"//android/player/src/test/java/android/util:android_utils_stub",
"//android/player/src/test/java/com/intuit/playerui/android/utils:android_test_utils",
"//android/player/src/test/java/com/intuit/playerui/android/utils:android_log_util"] + test_deps,
)

lint(
name = name,
srcs = native.glob(["**/*.kt"]),
lint_config = "//jvm:lint_config",
)
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import androidx.transition.TransitionManager
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext

internal suspend infix fun View?.intoOnMain(root: FrameLayout) = withContext(Dispatchers.Main) {
suspend infix fun View?.intoOnMain(root: FrameLayout) = withContext(Dispatchers.Main) {
into(root)
}

Expand Down
9 changes: 9 additions & 0 deletions android/player/src/test/java/android/util/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
load("//android/player:deps.bzl", "test_deps")

kt_jvm_library(
name = "android_utils_stub",
srcs = ["Log.kt"],
deps = ["//android/player:player"],
visibility = ["//visibility:public"]
)
24 changes: 11 additions & 13 deletions android/player/src/test/java/android/util/Log.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,47 @@

package android.util

internal var e: MutableList<String> = mutableListOf()
internal fun e(tag: String, msg: String): Int {
var e: MutableList<String> = mutableListOf()
fun e(tag: String, msg: String): Int {
val message = "ERROR: $tag: $msg"
e.add(message)
println(message)
return 0
}

internal var w: MutableList<String> = mutableListOf()
internal fun w(tag: String, msg: String): Int {
var w: MutableList<String> = mutableListOf()
fun w(tag: String, msg: String): Int {
val message = "WARN: $tag: $msg"
w.add(message)
println(message)
return 0
}

internal var i: MutableList<String> = mutableListOf()
internal fun i(tag: String, msg: String): Int {
var i: MutableList<String> = mutableListOf()
fun i(tag: String, msg: String): Int {
val message = "INFO: $tag: $msg"
i.add(message)
println(message)
return 0
}

internal var d: MutableList<String> = mutableListOf()
internal fun d(tag: String, msg: String): Int {
var d: MutableList<String> = mutableListOf()
fun d(tag: String, msg: String): Int {
val message = "DEBUG: $tag: $msg"
d.add(message)
println(message)
return 0
}

internal var v: MutableList<String> = mutableListOf()
internal fun v(tag: String, msg: String): Int {
var v: MutableList<String> = mutableListOf()
fun v(tag: String, msg: String): Int {
val message = "TRACE: $tag: $msg"
v.add(message)
println(message)
return 0
}

internal fun clearLogs() = listOf(e, w, i, d, v).forEach { it.clear() }

internal enum class Level {
enum class Level {
Error, Warn, Info, Debug, Verbose;

fun getLogs(): List<String> = when (this) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package com.intuit.playerui.android.renderer
package com.intuit.playerui.android

import android.content.Context
import android.widget.FrameLayout
import android.widget.LinearLayout
import com.intuit.playerui.android.AndroidPlayer
import com.intuit.playerui.android.AssetContext
import com.intuit.playerui.android.asset.StaleViewException
import com.intuit.playerui.android.utils.BrokenAsset
import com.intuit.playerui.android.utils.BrokenAsset.Companion.asset
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
load("//android/player:build.bzl", "kt_player_test")

kt_player_test(
name = "extensions-tests",
srcs = [
"CoroutineTestDispatcherExtension.kt",
"IntoKtTest.kt"
],
)

exports_files(["CoroutineTestDispatcherExtension.kt"])
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.intuit.playerui.android.logger

import android.util.clearLogs
import android.util.d
import android.util.e
import android.util.i
import android.util.v
import android.util.w
import com.intuit.playerui.android.utils.clearLogs
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
Expand Down Expand Up @@ -36,8 +36,6 @@ internal class AndroidLoggerTest {
assertInfo()
logger.warn(warn)
assertWarn()
logger.error(error)
assertError()
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("//android/player:build.bzl", "kt_player_test")

kt_player_test(
name = "logger-tests",
srcs = [
"AndroidLoggerTest.kt"
],
associates = ["//android/player:player-associates"]
)
Loading