do not merge ci: stop running the unit test suite twice per build - #385
Open
abelonogov-ld wants to merge 1 commit into
Open
do not merge ci: stop running the unit test suite twice per build#385abelonogov-ld wants to merge 1 commit into
abelonogov-ld wants to merge 1 commit into
Conversation
check pulled in both testDebugUnitTest and testReleaseUnitTest, so every CI run executed all 729 unit tests twice. The module has no variant-specific source sets, so the second pass compiled and ran identical code for no added coverage while doubling unit test time and doubling the chance that a run trips a flaky test. A recent PR failure landed in testReleaseUnitTest while the same flake failed a release in testDebugUnitTest. Disabling unit tests for the release variant leaves the release AAR, its publication, and lint untouched. Co-authored-by: Cursor <cursoragent@cursor.com>
abelonogov-ld
changed the title
| // doubles the chance of a run tripping a flaky test. Re-enable if a variant-specific difference | ||
| // (such as the BuildConfig.DEBUG branch in LDTimberLogging) ever gains unit test coverage. | ||
| beforeVariants(selector().withBuildType("release")) { variantBuilder -> | ||
| variantBuilder.enableUnitTest = false |
Contributor
There was a problem hiding this comment.
Wouldn't we prefer to disable unit tests for the debug build type and enable them for the release build type?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Follow-up to the investigation of why unit tests seem to fail more often during releases than on PRs. The short answer is that the unit test setup is identical in both paths —
ci.yml(pull requests and pushes) andpublish.yml(releases) both call.github/actions/ciwith the same inputs, which runs./gradlew build jarfollowed by./gradlew test. There is no release-specific test configuration.What the failure record actually shows:
StateDebounceManagerTest > timerResetsOnEachEventStateDebounceManagerTest > callbackNotFiredBeforeDebounceWindowFDv2DataSourceTest > recoveryResetsToFirstAvailableSynchronizer(OOM)The first two were wall-clock timing flakes, already made deterministic by #359 and its follow-up; the third is fixed in #384. PR runs hit the same flakes — run 31133027807 failed with the identical FDv2 OOM. Releases are simply where a flake is noticed, because it aborts publishing instead of being quietly re-run.
The one real defect found
checkpulls in bothtestDebugUnitTestandtestReleaseUnitTest, so every CI run executes all 729 unit tests twice. This module has no variant-specific source sets, so the second pass compiles and runs identical code. It buys no coverage and costs roughly a minute of CI per run, and more importantly it gives each run two independent chances to trip a flaky test. That asymmetry is visible in the data above: the recent PR failure landed intestReleaseUnitTestwhile the same flake failed the release intestDebugUnitTest.This disables unit tests for the release variant only.
Verification
./gradlew build jar --dry-runnow schedules:launchdarkly-android-client-sdk:testDebugUnitTestand no longertestReleaseUnitTest;assembleReleaseandbundleReleaseAarare still scheduled.:launchdarkly-android-client-sdk:checkand:assembleReleaseboth pass, and onlytestDebugUnitTestproduces results.publishToMavenLocalstill resolves, so the release artifact and its publication are untouched.testReleaseUnitTest.The comment in
build.gradlenotes the one place where variant behavior actually differs (theBuildConfig.DEBUGbranch inLDTimberLogging, currently untested), so this can be revisited if that ever gains coverage.Test plan
ci-buildpasses with a single unit test passNote
Overview
CI now runs the Android client SDK unit tests once per build by turning off unit tests for the release build variant in
launchdarkly-android-client-sdk/build.gradle.Gradle’s
checktask previously ran bothtestDebugUnitTestandtestReleaseUnitTest. This module has no variant-specific sources, so the second pass duplicated the same ~729 tests—adding roughly a minute of CI time and a second independent chance for flaky tests to fail. The change usesandroidComponents.beforeVariantswithenableUnitTest = falseon the release build type only; debug unit tests remain.Release assembly, AAR bundling, and Maven publication are unchanged. A comment documents re-enabling if variant-specific behavior (e.g.
BuildConfig.DEBUGinLDTimberLogging) ever needs release-variant test coverage.Reviewed by Cursor Bugbot for commit 1fb2b38. Bugbot is set up for automated code reviews on this repo. Configure here.