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

Enable plugin to skip variants/flavors #205

Closed
3dm1 opened this issue Jun 24, 2020 · 8 comments
Closed

Enable plugin to skip variants/flavors #205

3dm1 opened this issue Jun 24, 2020 · 8 comments

Comments

@3dm1
Copy link

3dm1 commented Jun 24, 2020

Using a project with two variants, debug/release, as an example. If some of my unit tests rely on classes defined on the debug variant only, assembleReleaseUnitTests will throw an error due to unreferenced classes. The proper approach would be to move those tests to a testDebug folder, but this becomes a problem in already complex projects.

Enabling the plugin to skip certain variants, e.g. release, allow users to still get all the information without having to fix certain builds issues.

@3dm1 3dm1 changed the title Enable plugin to skip variants Enable plugin to skip variants/flavors Jun 24, 2020
@autonomousapps
Copy link
Owner

Could you elaborate or, even better, provide a reproducer? I'd like to take a deeper look and see if there's some incorrect assumption I'm making, which I could address.

@3dm1
Copy link
Author

3dm1 commented Jun 24, 2020

I'll try work on a repro example. I believe my point boils down to the fact this plugin suffers as the number of build types and flavors grows. In a scenario where the project has 2 build types (debug and release) and two flavor (demo and full), the dependencies of all four combinations will be evaluated. Another flavor brings the total permutations to 6 and so forth.

This results in a longer period before buildHealth completes and prevents me to adopt it if some of my variants do not comply with some of the assumptions previously made.

@autonomousapps
Copy link
Owner

I agree that analyzing all variants leads to longer analysis, but this follows my philosophy for this plugin of making it simple to configure while also providing the most accurate advice possible. Ignoring certain variants automatically violates that second principle, while also violating the first in the sense that it complicates configuration, both from the user perspective and the in the implementation of the plugin itself.

I'm not really concerned about run time. I have one user with ~2000 modules and multiple variants, and even in their case, the initial (totally uncached ant not up to date) run took less than 20min. Very few projects are at that scale.

I'm certainly open to discussion, but I'm not inclined to add support for ignoring certain variants. As I said earlier, I'd prefer to fix the plugin (if possible) to handle scenarios like yours.

@3dm1
Copy link
Author

3dm1 commented Jun 25, 2020

I managed to create a minimal example that reproduces the issue and uploaded to Google Drive.
There's a ClassToTest under app/debug and it is tested on the generic test variant. Note that :app:compileReleaseUnitTestKotlin throws an unresolved class error but the IDE won't complain about this setup unless you select the release variant.

As I said before, I understand that this is not a bug on the plugin but, as it is, it prevents me from getting results without fixing the entire code structure. Moreover I might never expect to run unit tests on the release build type - using the example above - and therefore it is less relevant for me to fix issues on that variant.

Ignoring certain variants automatically violates that second principle

I absolutely don't want any automatic behavior, but rather the ability to configure the plugin to skip those if I feel like I'll get enough information by only running a subset of the available variants. Similarly to how skipping ktx may produce cleaner results for user, skipping variants will produce better results on my scenario.

@autonomousapps
Copy link
Owner

I requested access to the google drive link.

@autonomousapps
Copy link
Owner

autonomousapps commented Jun 29, 2020

This is a really interesting case. I'm not sure how best to handle this at the level of my plugin. One thing you can do right now, though, is add this to your build script:

android {
  onVariants {
    if (buildType == 'release') {
      unitTest {
        enabled = false
      }
    }
  }
}

this is available since AGP 4.0.0, I think. I have verified that adding this to your reproducer will allow buildHealth to complete.

Since your goal is "avoid processing some variants", it makes sense to me to use existing facilities rather than waiting for me to tack something on to this plugin. (This plugin will still analyze the release variant, but the time involved is negligible for most projects.)

I do not mean to disregard your position on the subject of customizing behavior. However, it's a complex design and philosophy question, and not quite as black and white as "let users do whatever they want." Your ktx example provides an interesting comparison to the current request. The existence of what I call "ktx" dependencies points to the existence of a real thing that simply is not yet modeled by the gradle build tool -- that is, a "logical dependency" that is the set of several related dependencies that are expected to be consumed together, despite their existence as separate artifacts. Your request in this issue does not point to the existence of some fundamental thing that is as yet un-modeled by existing tooling; rather, it is about convenience. Moreover, convenience at the cost of accuracy. I value accuracy very highly. In fact, I prefer to provide false negatives (not reporting potential issues) than false positives (incorrectly reporting non-existent issues). This is because false negatives don't break a build (they follow the principle of least-harm), whereas false positives do break builds.

All of that said (and I welcome debate), I will at least see if I can detect this situation and emit a more warning when the build fails. Not sure if this is possible, but we'll see.

@3dm1
Copy link
Author

3dm1 commented Jun 29, 2020

The suggestion partially worked, and I say that because on AGP v4 it fails with unitTestVariant must not be null, but that's is completely unrelated to your plugin. The important part is that skipping unit tests for unwanted variants is generally the solution to my problem.

I really appreciate you taking time to engage in this discussion and by now I understand better where you're coming from and with which priorities. That is all to say that I agree with your position on accuracy vs convenience in the context of this plugin and I thank you for creating it 😄

I don't think there's much more to be extracted from this issue, so I'll close it.

@3dm1 3dm1 closed this as completed Jun 29, 2020
@cosic
Copy link

cosic commented Dec 26, 2022

@autonomousapps Hi. Is it possible to apply the plugin only from single buildType or flavor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants