avito-tech/avito-android

UI Impact analysis through module pointers

dsvoronin opened this issue · 6 comments

Problem

Android gradle plugin 3.6 makes namespacedRClass on by default, so transitive resources doesn't merge up through modules graph.
It breaks our approach to map R.id constants with it's modules as id's are no longer statically resolved.

Proposal

We could add new field for com.avito.android.screen.Screen:

interface Screen {
    val rootId: Int
+   val module: String
}

rootId is still needed for isOpened method

module is a module path, like :lib:foo, it should point to the closest to app module containing this screen logic

Q: Will at be harder to support?
A: A bit, modules are more or less stable, but we lost isOpened guarantees (isOpened check kept rootId actual)

Q: What if someone renames module?
A: if module field point on nonexistent module we could fail build on analysis with descriptive error

Q: What if screen moved to another module?
A: Incorrect module pointers will lead to incorrect ui impact analysis results. Runtime check could be added to match computed rootId with module based on info from module's intermediates

Future

We are going to left a small(and decreasing) amount of e2e tests in app module, and move component tests to library modules, so this impact analysis approach will be less effective, as libraries inherits standard "per module"
impact analysis.

Why not remove all ui impact analysis code at all?

  1. We want to keep our PR times at current level before component tests goes to modules
  2. Some of it's logic could be reused, for example to find new and changed tests on PR

Can we get the module out from the project structure?

Can we get the module out from the project structure?

it should be a compile time constant, it theory we could generate file, like BuildConfig.java, but in practice i'd prefer not to do that. Too complex for most likely a temporary solution

Can we check the contract rootId + module automatically in runtime?

For all screens:

  1. Find the module
  2. Check that it contains rootId.
    Must it be available in this module?

Just thoughts.
How does AGP merge resources?
Does It get some intermediate artifacts? How does it map namespacedR to R?

Can we check the contract rootId + module automatically in runtime?

For all screens:

  1. Find the module
  2. Check that it contains rootId.
    Must it be available in this module?

yep, looks possible
we can add symbols list from all modules to androidTest resources
map it like {"moduleName" -> allIds}
and check on Screen initialization if this module contains this id (once, cached)

it will strenghten contract for sure, i'll update the proposal

Just thoughts.
How does AGP merge resources?
Does It get some intermediate artifacts? How does it map namespacedR to R?

intermediate artifacts are in module's build/intermediate:

symbol_list_with_package_name/<variant>/package-aware-r.txt
with content like:

com.avito.android.remote.notification
drawable ic_notification
id message
id refresh
layout notification_center_load_snippet
string notification_direct_reply

and
compile_only_not_namespaced_r_class_jar/<Variant>/R.java

now without actial ids (all is 0)

and all of it merged into app's runtime_symbol_list/<variant>/R.txt with actual id's