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

build(deps): fix build issues #11096

Merged
merged 6 commits into from Apr 26, 2022
Merged

Conversation

mikehardy
Copy link
Member

@mikehardy mikehardy commented Apr 25, 2022

Pull Request template

Purpose / Description

Each commit here does a thing in our build scripts -

1- a simple jacoco version update (dependabot doesn't see these for whatever reason)
2- using gradle-build-action in all our workflows (I trialed it in emulator tests, works well, Fixes #11080)
3- hamcrest is used in multiple modules, extract version, update it in lagging module
4- extract our build best practices so lint-rules gets -Werror compiler arg "for free" (Fixes #11083)
5- post-process androidTest XML to make sure non-zero # of tests ran (Fixes #11078)

Fixes

Linked above

Approach

How Has This Been Tested?

I tested the -Werror / "extract build common practices" by doing git revert a70c0f6a381b31ad205388d4b5e3d6e1f3198f15 then running the build - ✔️ got the expected error

mike@bistromath:~/work/AnkiDroid/Anki-Android (batch-of-build-work) % !2142
./gradlew clean jacocoTestReport ktlintCheck lint lint-rules:test --rerun-tasks

> Task :AnkiDroid:processPlayDebugMainManifest
/home/mike/work/AnkiDroid/Anki-Android/AnkiDroid/src/test/AndroidManifest.xml:39:9-45:51 Warning:
        provider#org.acra.attachment.AcraContentProvider@android:authorities was tagged at AndroidManifest.xml:39 to replace other declarations but no other declaration present

> Task :lint-rules:compileKotlin FAILED
e: warnings found and -Werror specified
w: /home/mike/work/AnkiDroid/Anki-Android/lint-rules/src/main/java/com/ichi2/anki/lint/rules/NonPositionalFormatSubstitutions.kt: (57, 28): The corresponding parameter in the supertype 'ResourceXmlDetector' is named 'folderType'. This may cause problems when calling this function with named arguments.

FAILURE: Build failed with an exception.

I tested the zero tests by starting android API21 locally by itself, API21 + API31, and API31 by itself. 31 by itself passes (:heavy_check_mark:) but any time 21 is in - by itself or in combo with others - :heavy_check_mark: get the expected error:

> Task :AnkiDroid:assertNonzeroAndroidTests FAILED

FAILURE: Build failed with an exception.

* Where:
Build file '/home/mike/work/AnkiDroid/Anki-Android/AnkiDroid/build.gradle' line: 216

* What went wrong:
androidTest executed 0 tests for TEST-TestingAVD_API-31_TAG-PLAY(AVD) - 12-AnkiDroid-play.xml - Probably a bug with the emulator. Try another image.

Learning (optional, can help others)

  • Groovy is pretty powerful
  • Project-level "afterEvaluate" is pretty powerful to extract common build elements
  • Gradle lifecycle task dependency features always have what you need

AnkiDroid/build.gradle Outdated Show resolved Hide resolved
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Always amazing seeing you do this.

A few questions, then let's get this in

AnkiDroid/jacoco.gradle Show resolved Hide resolved
.github/workflows/lint.yml Show resolved Hide resolved
lint-rules/build.gradle Show resolved Hide resolved
AnkiDroid/build.gradle Show resolved Hide resolved
AnkiDroid/build.gradle Outdated Show resolved Hide resolved
This should optimize LRU cache eviction so main and branches always have
reasonably fresh gradle caches and main does not ever get evicted, since
branches have independent caches and would otherwise fill up 10GB cache limit

Note that `--daemon` is specified as the default for this action is no daemon,
that is correct for long-lived runners to avoid cross-build contamination, but
on GitHub Hosted Runners (which we use) we are clean each run, so this is a small
optimization to share gradle between steps: gradle/gradle-build-action#113 (comment)
This is the recommended way to upgrade hamcrest from 1.x to 2.x when
using junit 4.x, see http://hamcrest.org/JavaHamcrest/distributables#gradle-upgrade-example
extract our best-practices build settings to project-level file, apply
to all sub-projects

Fixes ankidroid#11083
AnkiDroid/build.gradle Outdated Show resolved Hide resolved
@david-allison david-allison added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Review labels Apr 26, 2022
@david-allison
Copy link
Member

david-allison commented Apr 26, 2022

One confirmation I'd like is whether the "0 emulator" case was re-tested

@mikehardy
Copy link
Member Author

mikehardy commented Apr 26, 2022

Tested the snot out of it, my local API21 is a great test. So I redid tests of API21 solo, API21+API31, API31 solo, got expected results (well actually saw it was all messed up first, then got expected results with bonus debugging output to help verify which I will remove, good spot).
After doing nothing other than removing println, if it goes green I'll merge. Thanks for the help, definitely better for it

Sometimes an emulator "runs" tests, but it actually discovered zero
tests, runs zero tests, and still reports success. Lookin' at you API21.

Fixes ankidroid#11078
It appears upstream has fixed the leak as of 0.45.0, so workaround
not needed, and ktlint is now on 0.45.2 anyway, so use that

Can't remove the actual version override that gets us 0.45.x until
the ktlint gradle plugin has a new release but that hasn't happened yet
@mikehardy mikehardy merged commit 1f2dc01 into ankidroid:main Apr 26, 2022
@github-actions github-actions bot added this to the 2.16 release milestone Apr 26, 2022
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Apr 26, 2022
@mikehardy mikehardy deleted the batch-of-build-work branch April 26, 2022 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants