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.

Sign up for GitHub

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

Jump to bottom

applicationVariants/libraryVariants are eagerly evaluated and result in missed test data #362

Closed
ZacSweers opened this issue May 4, 2023 · 3 comments · Fixed by #406
Closed

applicationVariants/libraryVariants are eagerly evaluated and result in missed test data #362

ZacSweers opened this issue May 4, 2023 · 3 comments · Fixed by #406
Assignees
@shanshin
Labels
Bug Bug issue type S: ready for release Status: merged in the main branch

Comments

@ZacSweers
Copy link

Describe the bug
This code eagerly evaluates applicationVariants/libraryVariants in Android projects, and the result is that they have empty variants if the plugin is applied before the android variants are set up and the kover report task will be skipped due to no inputs.

kotlinx-kover/src/main/kotlin/kotlinx/kover/gradle/plugin/locators/Android.kt

Lines 28 to 32 in 9c36932

val variants = if ("applicationVariants" in androidExtension) {
androidExtension.propertyBeans("applicationVariants")
} else {
androidExtension.propertyBeans("libraryVariants")
}

Errors
If present, stacktraces or files from build/kover/errors directory

Expected behavior
This API is old and the wrong API to use. Instead, the new androidComponents.onVariants API should be used, as this is a push system that will callback with each variant. This API is present in AGP 7.0 and later.

Reproducer
Easy to reproduce in a project that applies the kover plugin before configuring android variants.

Reports
If applicable, report files or screenshots.

Environment

  • Kover Gradle Plugin version: 0.7.0-Beta
  • Gradle version: 8.0.2/8.1.1
  • Kotlin project type: Kotlin/Android
  • Coverage Engine version (if customized in build script): N/A
  • Other context important for this bug: [e.g. OS version]
@ZacSweers ZacSweers added Bug Bug issue type S: untriaged Status: issue reported but unprocessed labels May 4, 2023
@ZacSweers
Copy link
Author

I should mention I've verified the racy behavior by delaying applying the kover plugin behind an afterEvaluate block

@shanshin
Copy link
Collaborator

shanshin commented May 4, 2023

Great, thanks!

We will create improved locator for AGP >= 7.0

@shanshin shanshin added S: in progress Status: implementing or design in process and removed S: untriaged Status: issue reported but unprocessed labels May 4, 2023
ZacSweers added a commit to slackhq/slack-gradle-plugin that referenced this issue May 4, 2023
@ZacSweers
Work around kover bug
eb2c710 
Kotlin/kotlinx-kover#362
@ZacSweers ZacSweers mentioned this issue May 4, 2023
Merged
ZacSweers added a commit to slackhq/slack-gradle-plugin that referenced this issue May 4, 2023
@ZacSweers
Work around kover bug (#349)
6273167 
Kotlin/kotlinx-kover#362

<!--
  ⬆ Put your description above this! ⬆

  Please be descriptive and detailed.
  
Please read our [Contributing
Guidelines](https://github.com/tinyspeck/slack-gradle-plugin/blob/main/.github/CONTRIBUTING.md)
and [Code of Conduct](https://slackhq.github.io/code-of-conduct).

Don't worry about deleting this, it's not visible in the PR!
-->
ZacSweers pushed a commit to ZacSweers/CatchUp that referenced this issue May 6, 2023
@renovate
Update sgp to v0.9.3 (#674)
88c1b74 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[com.slack.gradle.root](https://togithub.com/slackhq/slack-gradle-plugin)
| `0.9.0` -> `0.9.3` |
[![age](https://badges.renovateapi.com/packages/maven/com.slack.gradle.root/0.9.3/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/maven/com.slack.gradle.root/0.9.3/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/maven/com.slack.gradle.root/0.9.3/compatibility-slim/0.9.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/maven/com.slack.gradle.root/0.9.3/confidence-slim/0.9.0)](https://docs.renovatebot.com/merge-confidence/)
|
|
[com.slack.gradle.base](https://togithub.com/slackhq/slack-gradle-plugin)
| `0.9.0` -> `0.9.3` |
[![age](https://badges.renovateapi.com/packages/maven/com.slack.gradle.base/0.9.3/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/maven/com.slack.gradle.base/0.9.3/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/maven/com.slack.gradle.base/0.9.3/compatibility-slim/0.9.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/maven/com.slack.gradle.base/0.9.3/confidence-slim/0.9.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### ⚠ Dependency Lookup Warnings ⚠

Warnings were logged while processing this repo. Please check the
Dependency Dashboard for more information.

---

### Release Notes

<details>
<summary>slackhq/slack-gradle-plugin</summary>

###
[`v0.9.3`](https://togithub.com/slackhq/slack-gradle-plugin/blob/HEAD/CHANGELOG.md#&#8203;093)

[Compare
Source](https://togithub.com/slackhq/slack-gradle-plugin/compare/0.9.2...0.9.3)

*2023-05-05*

- Add `jdk.compiler/com.sun.tools.javac.model` to Bootstrap Gradle JVM
args and exec prefixes for binaries for GJF 17.

###
[`v0.9.2`](https://togithub.com/slackhq/slack-gradle-plugin/blob/HEAD/CHANGELOG.md#&#8203;092)

[Compare
Source](https://togithub.com/slackhq/slack-gradle-plugin/compare/0.9.1...0.9.2)

*2023-05-05*

-   Fix accidental noisy JVM vendor log.

###
[`v0.9.1`](https://togithub.com/slackhq/slack-gradle-plugin/blob/HEAD/CHANGELOG.md#&#8203;091)

[Compare
Source](https://togithub.com/slackhq/slack-gradle-plugin/compare/0.9.0...0.9.1)

*2023-05-04*

Happy May the Fourth!

- Add new `sgp.config.jvmVendor` property to control the JVM vendor used
in Kotlin and Java toolchains. This value is used to match a known
vendor spec, such as `AZUL`.
- Apply the kover plugin in an `afterEvaluate` block to avoid
[Kotlin/kotlinx-kover#362.
-   Update jgrapht to 1.5.2.
-   Update oshi to 6.4.2.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/ZacSweers/CatchUp).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS43MS4wIiwidXBkYXRlZEluVmVyIjoiMzUuNzEuNCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@shanshin shanshin mentioned this issue May 10, 2023
Closed
@sabufung30 sabufung30 mentioned this issue May 27, 2023
Closed
mikescamell added a commit to mikescamell/purchases-android that referenced this issue May 29, 2023
@mikescamell
Apply kover after project evaluation
6a6fb37 
There's currently a bug in 0.7.0 with Android projects where it reports it's skipping generation as not tests found:Kotlin/kotlinx-kover#362

This is a temporary workaround.
mikescamell added a commit to mikescamell/purchases-android that referenced this issue May 29, 2023
@mikescamell
Apply kover after project evaluation
7018f25 
There's currently a bug in 0.7.0 with Android projects where it reports it's skipping generation as not tests found:Kotlin/kotlinx-kover#362

This is a temporary workaround.
This was referenced May 31, 2023
Closed
Closed
This was referenced Jun 5, 2023
Closed
Closed
@outadoc outadoc mentioned this issue Jun 9, 2023
Closed
shanshin added a commit that referenced this issue Jun 9, 2023
@shanshin
Removed the dependency on the order of applying of the plugin
f5a3ae1 
Now the search for build variants takes place in the `afterEvaluate` action, which is added when `finalizeDsl` is called - thus the Kover code is guaranteed to be executed after creating variants.

Additional changes:
* added DSL accessors for Kover Default report tasks
* added descriptions for Kover report tasks
* added functions for getting Kover Android report task names

Fixes #362
Fixes #394
Fixes #400
@shanshin shanshin mentioned this issue Jun 9, 2023
Merged
@shanshin shanshin reopened this Jun 16, 2023
@shanshin shanshin added S: ready for release Status: merged in the main branch and removed S: in progress Status: implementing or design in process labels Jun 16, 2023
@shanshin
Copy link
Collaborator

Fixed in 0.7.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shanshin shanshin

Labels
Bug Bug issue type S: ready for release Status: merged in the main branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Removed the dependency on the order of applying of the plugin Kotlin/kotlinx-kover
2 participants
@ZacSweers @shanshin