From dcf004f80ab49d55b98ab71af0061df674d55aef Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 21 Aug 2026 15:03:32 -0700 Subject: [PATCH 1/4] =?UTF-8?q?ADFA-4128:=20qb=2005/12=20core-detection=20?= =?UTF-8?q?=E2=80=94=20Core=20slice=201:=20watch=20the=20tree,=20coalesce?= =?UTF-8?q?=20a=20save=20burst,=20classify=20the=20cheapest=20correct=20ro?= =?UTF-8?q?ute?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W --- quickbuild/core/.gitignore | 1 + quickbuild/core/README.md | 139 +++++ quickbuild/core/build.gradle.kts | 77 +++ quickbuild/core/consumer-rules.pro | 0 quickbuild/core/proguard-rules.pro | 0 quickbuild/core/src/main/AndroidManifest.xml | 17 + .../quickbuild/data/AndroidProjectWatcher.kt | 308 +++++++++++ .../cotg/quickbuild/data/ProjectWatcher.kt | 28 + .../cotg/quickbuild/domain/ChangedFiles.kt | 71 +++ .../domain/annotations/AnnotationBaseline.kt | 84 +++ .../domain/annotations/AnnotationFacts.kt | 49 ++ .../domain/annotations/AnnotationImpact.kt | 152 ++++++ .../annotations/AnnotationProcessorProfile.kt | 270 ++++++++++ .../quickbuild/domain/annotations/README.md | 11 + .../annotations/SourceAnnotationScanner.kt | 377 +++++++++++++ .../quickbuild/domain/classify/BuildRoute.kt | 131 +++++ .../domain/classify/ChangeClassifier.kt | 300 +++++++++++ .../cotg/quickbuild/domain/classify/README.md | 8 + .../domain/classify/TestSourceFilter.kt | 87 +++ .../domain/watch/ChangeCoalescing.kt | 143 +++++ .../cotg/quickbuild/domain/watch/README.md | 9 + .../quickbuild/domain/watch/WatchFilter.kt | 102 ++++ .../domain/watch/WatcherBatchReconciler.kt | 46 ++ .../quickbuild/OfflineNetworkGuardTest.kt | 55 ++ .../data/AndroidProjectWatcherEdgeTest.kt | 138 +++++ .../data/AndroidProjectWatcherTest.kt | 210 ++++++++ .../quickbuild/domain/ChangedFilesTest.kt | 80 +++ .../AnnotationImpactAnalyzerTest.kt | 507 ++++++++++++++++++ .../AnnotationProcessorProfileTest.kt | 98 ++++ .../domain/annotations/RoomAppFixture.kt | 169 ++++++ .../SourceAnnotationScannerEdgeTest.kt | 309 +++++++++++ .../SourceAnnotationScannerTest.kt | 219 ++++++++ .../domain/classify/BuildRouteTest.kt | 42 ++ .../classify/ChangeClassifierEdgeTest.kt | 78 +++ .../domain/classify/ChangeClassifierTest.kt | 411 ++++++++++++++ .../domain/classify/TestSourceFilterTest.kt | 178 ++++++ .../domain/watch/ChangeCoalescingEdgeTest.kt | 46 ++ .../domain/watch/ChangeCoalescingTest.kt | 220 ++++++++ .../domain/watch/WatchFilterTest.kt | 118 ++++ .../watch/WatcherBatchReconcilerTest.kt | 127 +++++ settings.gradle.kts | 1 + 41 files changed, 5416 insertions(+) create mode 100644 quickbuild/core/.gitignore create mode 100644 quickbuild/core/README.md create mode 100644 quickbuild/core/build.gradle.kts create mode 100644 quickbuild/core/consumer-rules.pro create mode 100644 quickbuild/core/proguard-rules.pro create mode 100644 quickbuild/core/src/main/AndroidManifest.xml create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/ProjectWatcher.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/ChangedFiles.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationBaseline.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationFacts.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfile.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/README.md create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRoute.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifier.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilter.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescing.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/README.md create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilter.kt create mode 100644 quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconciler.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/OfflineNetworkGuardTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/ChangedFilesTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfileTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/RoomAppFixture.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerEdgeTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRouteTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierEdgeTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilterTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingEdgeTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilterTest.kt create mode 100644 quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconcilerTest.kt diff --git a/quickbuild/core/.gitignore b/quickbuild/core/.gitignore new file mode 100644 index 0000000000..796b96d1c4 --- /dev/null +++ b/quickbuild/core/.gitignore @@ -0,0 +1 @@ +/build diff --git a/quickbuild/core/README.md b/quickbuild/core/README.md new file mode 100644 index 0000000000..5dbb4219ac --- /dev/null +++ b/quickbuild/core/README.md @@ -0,0 +1,139 @@ +# `:quickbuild:core` - the IDE-side half of Quick Build + +Decides *what* to do on every save and drives the session that does it: watch the project, +classify each change into a [`BuildRoute`](src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRoute.kt), +run the live reload path or hand back to Gradle, and deploy the result to the running proxy app. + +For what Quick Build is and how the whole loop fits together, read [`../README.md`](../README.md) +first. This file only covers what is inside this module. + +## The one rule that shapes everything here + +**The domain layer is the Android-free floor.** Nothing under `domain/` imports `android.*` or +`androidx.*`, and nothing there takes a `Context`. Every Android capability the module needs is +declared as an interface - a *port* - and implemented in `:app`, wired in one Koin module +([`QuickBuildModule.kt`](../../app/src/main/java/com/itsaky/androidide/di/QuickBuildModule.kt)). + +**The module as a whole is not Android-free, and is not meant to be.** It is a +`com.android.library` with AIDL, and six files under `data/` and `service/` import `android.*` +where the implementation is inherently framework-bound - `FileObserver` in +[`AndroidProjectWatcher`](src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt), +`Service` and `Binder` in the deploy channel, `ComponentCallbacks2` for memory pressure. Those are +adapters at the edge, not logic. + +Two things follow from an Android-free `domain/`, and both are the point: + +- The routing rules, the session state machine and the deploy policy are **unit-testable on the + JVM** with no device and no Robolectric. That is most of `src/test/`. +- Swapping how CoGo installs an APK, watches files or reports metrics does not touch `domain/`. + +Adding a dependency on an Android type inside `domain/` breaks both. Add a port instead. + +## Packages + +Three layers, and dependencies flow **down** toward `domain/`. Nothing depends upward. Within +`domain/` and `service/` the sub-packages name the concern, and they line up: the `service/` +sub-package acts on what the `domain/` one of the same name decides. + +```mermaid +flowchart TB + subgraph service["service/ - runs the session, performs outside-world effects"] + direction LR + svcProvision["provision"] + svcSession["session"] + svcDeploy["deploy"] + svcTelemetry["telemetry"] + end + + subgraph data["data/ - ports (file watch, device paths, daemon); implemented in :app"] + direction LR + dataPorts["data"] + end + + subgraph domain["domain/ - pure logic and value types; the floor, depends on nothing above"] + direction LR + domWatch["watch"] + domClassify["classify"] + domSession["session"] + domReload["reload"] + domTelemetry["telemetry"] + domAnnotations["annotations"] + end + + %% within service: components call each other freely + svcProvision -->|"hands off the built LiveSession"| svcSession + svcSession -->|"sends compiled payloads"| svcDeploy + svcDeploy -->|"relaunches / reconnects the proxy"| svcProvision + + %% within domain: value types reference each other + domWatch -->|"a coalesced change batch"| domClassify + domClassify -->|"annotation-processor impact?"| domAnnotations + domReload -->|"which BuildRoute to run"| domClassify + domSession -->|"reads a BuildDiagnostic"| domReload + + %% cross-layer: everything points DOWN into domain, never back up + svcSession ==>|"runs the SessionReducer"| domSession + svcSession ==>|"drives the reload orchestrator"| domReload + svcProvision ==>|"tracks generations, real-id install"| domReload + svcDeploy ==>|"acts on the DeployDecision"| domReload + svcTelemetry ==>|"stamps the E2eTimeline"| domTelemetry + dataPorts ==>|"emits WatchEvents, applies WatchFilter"| domWatch + dataPorts ==>|"reads / writes the GenerationStore"| domReload +``` + +Thin arrows are references **within** a layer, which are allowed: `service/` components call each +other, `domain/` value types reference each other. Thick arrows (`==>`) cross layers, and every one +points **down** into `domain/`. The two directions review must reject are **`domain/ -> service/`** +and **`domain/ -> data/`** - the pure-logic floor never reaches up to effects or ports. Edge labels +name what each dependency carries; the diagram shows the principal edges, and the per-package tables +below carry the full file-level detail. + +`domain/` - pure logic and value types. `ChangedFiles`, the batch every layer speaks in, sits at +the root because it belongs to no single concern. + +| Package | Holds | Start reading at | +| --- | --- | --- | +| [`domain/watch/`](src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/) | what counts as a change: the debounce, the filter, the batch reconciler | [`ChangeCoalescing`](src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescing.kt), [`WatchFilter`](src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilter.kt) | +| [`domain/classify/`](src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/) | which route a batch takes, and why a baseline stops being trustworthy | [`ChangeClassifier`](src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifier.kt), [`BuildRoute`](src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRoute.kt) | +| [`domain/session/`](src/main/java/org/appdevforall/cotg/quickbuild/domain/session/) | the state machine: states, events, effects, and what the user is told | [`SessionReducer`](src/main/java/org/appdevforall/cotg/quickbuild/domain/session/SessionReducer.kt), [`QuickBuildSessionState`](src/main/java/org/appdevforall/cotg/quickbuild/domain/session/QuickBuildSessionState.kt) | +| [`domain/reload/`](src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/) | the live reload path: what to rebuild, hot swap versus restart, generations | [`LiveReloadOrchestrator`](src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/LiveReloadOrchestrator.kt), [`DeployPolicy`](src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/DeployPolicy.kt) | +| [`domain/telemetry/`](src/main/java/org/appdevforall/cotg/quickbuild/domain/telemetry/) | the measurement vocabulary: one timeline per edit, one sink to report it | [`E2eTimeline`](src/main/java/org/appdevforall/cotg/quickbuild/domain/telemetry/E2eTimeline.kt) | +| [`domain/annotations/`](src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/) | whether a change feeds an annotation processor, and what that costs | [`AnnotationImpact`](src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt) | + +| Package | Holds | Start reading at | +| --- | --- | --- | +| [`data/`](src/main/java/org/appdevforall/cotg/quickbuild/data/) | the ports themselves: file watching, device paths, the daemon process | `ProjectWatcher`, `QuickBuildPaths`, `DaemonProcessClient` | + +`service/` - session lifecycle and the effects that touch the outside world. + +| Package | Holds | Start reading at | +| --- | --- | --- | +| [`service/provision/`](src/main/java/org/appdevforall/cotg/quickbuild/service/provision/) | getting a proxy app built, installed and launched - including the clobber check | [`QuickBuildProvisioner`](src/main/java/org/appdevforall/cotg/quickbuild/service/provision/QuickBuildProvisioner.kt), [`ProxyAppInstaller`](src/main/java/org/appdevforall/cotg/quickbuild/service/provision/ProxyAppInstaller.kt) | +| [`service/deploy/`](src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/) | the AIDL channel to the proxy app and everything sent over it | [`PayloadDeployer`](src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/PayloadDeployer.kt), [`DeployChannel`](src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/DeployChannel.kt) | +| [`service/session/`](src/main/java/org/appdevforall/cotg/quickbuild/service/session/) | the session itself: holds the reducer, runs the effects, drives one build at a time | [`QuickBuildSessionManager`](src/main/java/org/appdevforall/cotg/quickbuild/service/session/QuickBuildSessionManager.kt), [`LiveReloadExecutorImpl`](src/main/java/org/appdevforall/cotg/quickbuild/service/session/LiveReloadExecutorImpl.kt) | +| [`service/telemetry/`](src/main/java/org/appdevforall/cotg/quickbuild/service/telemetry/) | stamping a timeline as a build runs, and reporting it | [`E2eTimelineRecorder`](src/main/java/org/appdevforall/cotg/quickbuild/service/telemetry/E2eTimelineRecorder.kt) | + +The split that matters: **`domain/` decides, `service/` acts.** A pure reducer computes the next +state and a list of effects; the session manager executes them. If you find yourself doing IO in +`domain/`, the logic wants to move to `service/` or the IO wants to become a port. + +## Two invariants that are easy to break + +- **Everything stateful runs on one dispatcher, and it must be single-threaded.** Effects are + `launch`ed rather than run inline so a dispatch can never re-enter itself. +- **The reducer is total.** An unknown `(state, event)` pair keeps the current state and produces + no effects, so a late or duplicate event cannot corrupt a session. Adding a state or event + without extending the reducer silently gets you this fallback, not a compile error. + +## Where the rest is + +| For | Read | +| --- | --- | +| What Quick Build is, the loop, the decisions | [`../README.md`](../README.md) | +| Which file implements which pipeline step | [`../docs/pipeline.md`](../docs/pipeline.md) | +| My edit did not show up - where to look | [`../docs/debugging.md`](../docs/debugging.md) | +| The wire formats this module speaks | [`../protocol/README.md`](../protocol/README.md) | + +The other halves of the feature live in sibling modules: [`../daemon/`](../daemon/) compiles, +[`../runtime/`](../runtime/) runs inside the proxy app, and +[`../../gradle-plugin/`](../../gradle-plugin/) builds it. diff --git a/quickbuild/core/build.gradle.kts b/quickbuild/core/build.gradle.kts new file mode 100644 index 0000000000..7a815d5632 --- /dev/null +++ b/quickbuild/core/build.gradle.kts @@ -0,0 +1,77 @@ +import com.itsaky.androidide.build.config.BuildConfig + +plugins { + alias(libs.plugins.android.library) + alias(libs.plugins.kotlin.android) +} + +android { + namespace = "${BuildConfig.PACKAGE_NAME}.quickbuild" + + buildFeatures.aidl = true + + // AndroidProjectWatcherTest constructs the real watcher on the JVM: FileObserver's + // stubs then no-op (inotify inert) while the poll/coalesce pipeline runs for real. + testOptions.unitTests.isReturnDefaultValues = true + + sourceSets { + named("main") { + // The deploy-channel AIDL lives in :quickbuild:runtime (the proxy app side). + // Compile the SAME .aidl here instead of depending on that module: its + // manifest declares the proxy app's appComponentFactory, which must never + // merge into CoGo's own APK. + aidl.srcDir("../runtime/src/main/aidl") + } + } +} + +tasks.withType { + useJUnitPlatform() +} + +// DoD coverage gate: >=90% line+branch on non-UI (domain/data) code. +// The root build attaches the jacoco agent to every Test task; for Android modules +// the exec lands at build/outputs/unit_test_code_coverage/UnitTest/, NOT +// build/jacoco/ -- a JacocoReport pointed at build/jacoco/ silently SKIPs and the +// gate is never measured (see docs/process learnings, ADFA-3834). +tasks.register("jacocoTestReport") { + group = "verification" + description = "JaCoCo line+branch coverage for the v8Debug unit tests." + dependsOn("testV8DebugUnitTest") + + reports { + xml.required.set(true) + html.required.set(true) + } + + // The javac output holds only generated code (AIDL stubs + BuildConfig), so the + // hand-written surface is exactly the Kotlin classes. + classDirectories.setFrom( + fileTree(layout.buildDirectory.dir("tmp/kotlin-classes/v8Debug")) { + exclude("**/BuildConfig*") + }, + ) + sourceDirectories.setFrom(files("src/main/java")) + executionData.setFrom( + layout.buildDirectory.file( + "outputs/unit_test_code_coverage/v8DebugUnitTest/testV8DebugUnitTest.exec", + ), + ) +} + +dependencies { + implementation(projects.logger) + implementation(projects.eventbusEvents) + // Wire DTOs/constants shared with the daemon (single protocol definition). + implementation(projects.quickbuild.protocol) + + implementation(libs.common.kotlin.coroutines.android) + implementation(libs.google.gson) + + testImplementation(libs.tests.junit.jupiter) + testImplementation(libs.tests.google.truth) + testImplementation(libs.tests.kotlinx.coroutines) + // Shared offline-guard scanner (OfflineNetworkGuardTest). + testImplementation(testFixtures(projects.quickbuild.protocol)) + testRuntimeOnly(libs.tests.junit.platformLauncher) +} diff --git a/quickbuild/core/consumer-rules.pro b/quickbuild/core/consumer-rules.pro new file mode 100644 index 0000000000..e69de29bb2 diff --git a/quickbuild/core/proguard-rules.pro b/quickbuild/core/proguard-rules.pro new file mode 100644 index 0000000000..e69de29bb2 diff --git a/quickbuild/core/src/main/AndroidManifest.xml b/quickbuild/core/src/main/AndroidManifest.xml new file mode 100644 index 0000000000..edc60f1208 --- /dev/null +++ b/quickbuild/core/src/main/AndroidManifest.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt new file mode 100644 index 0000000000..0220343aa5 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt @@ -0,0 +1,308 @@ +package org.appdevforall.cotg.quickbuild.data + +import android.os.FileObserver +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.consumeAsFlow +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.watch.ChangeCoalescingDefaults +import org.appdevforall.cotg.quickbuild.domain.watch.WatchEvent +import org.appdevforall.cotg.quickbuild.domain.watch.WatchFilter +import org.appdevforall.cotg.quickbuild.domain.watch.coalesceChanges +import org.slf4j.LoggerFactory +import java.io.File + +/** + * Watches an Android project's files on-device, reporting each settled burst as one batch: + * raw events -> [WatchFilter] -> [coalesceChanges] debounce -> one batch. Runs on [scope]. + * + * Hybrid by necessity: the project lives on sdcardfs/FUSE, which can drop inotify events under + * load, so [FileObserver] gives the low-latency path and a [pollIntervalMillis] mtime sweep + * bounds staleness when events are lost. + * + * @property watchedRoots directory trees walked recursively for both the inotify watches and + * the poll sweep; entries that are not directories are skipped rather than failing. + * @property watchedFiles individual files outside [watchedRoots] (gradle config and kin), + * covered by the poll alone - no inotify watch is registered on their parent directories. + * @property filter relevance test applied to every raw event before coalescing; drops build + * intermediates and editor temp files. + * @property scope coroutine scope the pipeline and poll jobs run in; cancelling it stops the + * watcher as surely as [stop] does. + * @property pollIntervalMillis delay in milliseconds between mtime+size sweeps - the upper + * bound on staleness when inotify drops an event. + * @property quietMillis idle gap in milliseconds that ends a burst (see [coalesceChanges]). + * @property maxMillis cap in milliseconds on how long one burst may keep accumulating before + * it is emitted regardless of quiet time. + * @property pollDispatcher where the recurring stat walk runs; blocking IO, so it must stay off + * the session manager's single-threaded ordering dispatcher. + */ +class AndroidProjectWatcher( + private val watchedRoots: List, + private val watchedFiles: List, + private val filter: WatchFilter, + private val scope: CoroutineScope, + private val pollIntervalMillis: Long = DEFAULT_POLL_MILLIS, + private val quietMillis: Long = ChangeCoalescingDefaults.QUIET_MILLIS, + private val maxMillis: Long = ChangeCoalescingDefaults.MAX_MILLIS, + private val pollDispatcher: CoroutineDispatcher = Dispatchers.IO, +) : ProjectWatcher { + // Unlimited so a burst of inotify events never blocks or drops on a slow drain - coalescing + // downstream collapses the flood into one batch per burst. + private val rawEvents = Channel(Channel.UNLIMITED) + private val observers = mutableListOf() + private var pipelineJob: Job? = null + private var pollJob: Job? = null + + /** + * Change fingerprints (path -> lastModified xor size), written by both inotify and the poll + * but consulted only by the poll, so a change inotify already delivered is not built twice. + * inotify must NOT gate on them: a same-length rewrite inside one mtime tick, or a tool that + * preserves mtime like `adb push`, collides and would be missed. Concurrent because both + * writers race; a lost race costs one harmless extra build. + */ + private val fingerprints = java.util.concurrent.ConcurrentHashMap() + + /** + * Starts the coalescing pipeline, registers an inotify observer per watched directory, and + * launches the poll sweep. + * + * @param onBatch invoked once per settled burst on [scope], after restamping; must not block, + * since it runs inline on the collecting coroutine. + */ + override fun start(onBatch: (ChangedFiles.Known) -> Unit) { + pipelineJob = + scope.launch { + rawEvents + .consumeAsFlow() + .filter { filter.isRelevant(it.file) } + .coalesceChanges(quietMillis, maxMillis) + .collect { batch -> + restampSettled(batch) + onBatch(batch) + } + } + + watchedRoots.filter(File::isDirectory).forEach { root -> + root.walkTopDown().filter(File::isDirectory).forEach(::observe) + } + // Snapshot before starting: an already-started observer's CREATE handler can + // append to [observers] concurrently, which would throw + // ConcurrentModificationException in a live iteration. + val initial = synchronized(observers) { observers.toList() } + initial.forEach(FileObserver::startWatching) + + pollJob = scope.launch(pollDispatcher) { pollLoop() } + log.info("Project watcher started: {} inotify dirs + {}ms poll", observers.size, pollIntervalMillis) + } + + /** Cancels both jobs, stops and drops every observer, and closes the raw-event channel. */ + override fun stop() { + pollJob?.cancel() + pollJob = null + pipelineJob?.cancel() + pipelineJob = null + synchronized(observers) { + observers.forEach(FileObserver::stopWatching) + observers.clear() + } + rawEvents.close() + } + + /** + * Registers an inotify observer for one directory; subdirectories created later get their own. + * + * @param dir the directory to watch; the observer is appended to [observers] unstarted, and + * the caller starts it. + */ + private fun observe(dir: File) { + synchronized(observers) { observers.add(newObserver(dir)) } + } + + /** + * Builds one directory's observer. + * + * One factory for every watch, whether registered at start or for a directory that appeared + * mid-session: the CREATE recursion has to be in all of them, or a tree created inside a + * mid-session directory gets no watch below its top level and its files fall back to the poll. + * + * @param dir the directory this observer reports for; unstarted, so the caller starts it. + * @return the observer, not yet watching and not yet in [observers]. + */ + @Suppress("DEPRECATION") // FileObserver(File,...) is API 29+; minSdk is 28 (B5 targets 28/29). + private fun newObserver(dir: File): FileObserver = + object : FileObserver(dir.absolutePath, EVENT_MASK) { + override fun onEvent( + event: Int, + path: String?, + ) { + if (path == null) return + val changed = File(dir, path) + // A new directory (new package, git checkout) needs its own watch, or + // files created inside it later are invisible to inotify. + if (event and CREATE != 0 && changed.isDirectory) { + registerCreatedTree(changed) + } + if (event and DELETE_MASK != 0) { + reportDeletion(changed) + } else { + report(changed, fromPoll = false) + } + } + } + + /** + * Watches [dir] and every directory beneath it, then starts them. + * + * Recursive because a tree can arrive whole - a git checkout, an unzip - and watching only its + * top level leaves everything deeper on the poll path. + * + * @param dir the newly created directory; the walk includes it. + */ + internal fun registerCreatedTree(dir: File) { + synchronized(observers) { + // Built fully, then published, so a live iteration of observers never sees a + // half-built batch. + val fresh = arrayListOf() + dir.walkTopDown().filter(File::isDirectory).forEach { d -> fresh.add(newObserver(d)) } + fresh.forEach(FileObserver::startWatching) + observers.addAll(fresh) + } + } + + /** Live inotify watch count, so a test can assert the CREATE recursion registered a whole tree. */ + internal fun watchCount(): Int = synchronized(observers) { observers.size } + + /** + * Sweeps the watched roots on a timer - the safety net that catches whatever inotify + * dropped, bounding staleness to one interval. Only stats files, never reads them. + */ + private suspend fun pollLoop() { + initFingerprints() // prime without firing: current on-disk state is the baseline + while (scope.isActive) { + delay(pollIntervalMillis) + sweep() + } + } + + /** + * Runs one mtime+size sweep: modifications and creations via [report], then deletions as + * the set difference between the paths [fingerprints] tracks and what this walk saw, each + * one re-stat'd before it is believed. That diff is the reliable deletion floor on sdcardfs, + * where inotify DELETE can be dropped, but it is only a CANDIDATE list: inotify writes + * [fingerprints] concurrently, so a file created after this walk passed its directory is in + * the map and absent from the walk while very much alive. + * The `filterTo` copy is required - [reportDeletion] mutates the map being walked. + * Internal so tests can drive one sweep instead of racing the timer. + */ + internal fun sweep() { + val current = HashSet() + forEachWatchedFile { file -> + current.add(file.absolutePath) + report(file, fromPoll = true) + } + fingerprints.keys + .filterTo(ArrayList()) { it !in current } + .forEach { path -> + val file = File(path) + // Only a stat proves a deletion. Reporting one for a live file drops its + // fingerprint and, because coalescing is last-event-wins, collapses a real + // save into a removal - handing the daemon a file it is still compiling and + // re-emitting the same change on the next sweep. isFile, not exists, so a path + // that turned into a directory still counts as vanished. + if (!file.isFile) reportDeletion(file) + } + } + + /** + * Re-records each delivered file's fingerprint once the batch has settled, so the next poll + * sweep does not re-emit it as a phantom second batch (one save, two builds). Stamps taken + * inside an inotify callback can be stale - `adb push` rewrites mtime after the CLOSE_WRITE + * that fingerprinted it. A later real write is still never missed: its own event emits + * unconditionally, and a dropped event leaves a stamp differing from the one recorded here. + * + * @param batch the coalesced set about to be handed to the caller; only its still-existing + * regular files are restamped, and [ChangedFiles.Known.removed] is deliberately untouched. + */ + private fun restampSettled(batch: ChangedFiles.Known) { + batch.files.forEach { file -> + if (file.isFile) { + fingerprints[file.absolutePath] = file.lastModified() xor file.length() + } + } + } + + /** + * Fingerprints a live file and emits it - the one choke point for both inotify and the poll. + * Only the poll gates emission on the fingerprint (see [fingerprints] for why inotify must + * not). Directories are dropped: never a compile input, and routing one to the classifier + * would wrongly trip a full rebaseline. Deletions must take the separate [reportDeletion] + * path, or the `!isFile` guard here would swallow them. Internal so tests can drive it. + * + * @param file the path that changed; ignored unless it is an existing regular file, so a + * directory or an already-deleted path is a no-op. + * @param fromPoll true when the mtime sweep found it, which emits only if the fingerprint + * actually moved; false for an inotify delivery, which always emits. + */ + internal fun report( + file: File, + fromPoll: Boolean, + ) { + if (!file.isFile) return + val stamp = file.lastModified() xor file.length() + val previous = fingerprints.put(file.absolutePath, stamp) + if (!fromPoll || previous != stamp) { + rawEvents.trySend(WatchEvent.Modified(file)) + } + } + + /** + * Emits a [WatchEvent.Removed] for a path we were actually tracking. Gating on the + * [fingerprints] removal makes it fire exactly once whether inotify or the poll notices + * first, and skips paths never tracked (a subdir, or a temp created and gone between + * sweeps). Whether a removal is real work or noise is decided downstream. + * + * @param file the vanished path; ignored unless [fingerprints] was tracking it. + */ + private fun reportDeletion(file: File) { + if (fingerprints.remove(file.absolutePath) != null) { + rawEvents.trySend(WatchEvent.Removed(file)) + } + } + + /** Stamps the current on-disk state as the poll's baseline, without emitting any event. */ + private fun initFingerprints() { + forEachWatchedFile { f -> fingerprints[f.absolutePath] = f.lastModified() xor f.length() } + } + + /** + * Visits every regular file currently under [watchedRoots], then each existing entry of + * [watchedFiles]. + * + * @param action called per file, not deduplicated - a [watchedFiles] entry that also sits + * under a watched root is visited twice. + */ + private inline fun forEachWatchedFile(action: (File) -> Unit) { + watchedRoots.filter(File::isDirectory).forEach { root -> + root.walkTopDown().filter(File::isFile).forEach(action) + } + watchedFiles.filter(File::isFile).forEach(action) + } + + private companion object { + private val log = LoggerFactory.getLogger("QB-ProjectWatcher") + private const val DEFAULT_POLL_MILLIS = 2_000L + + /** Deletion bits: a file removed from, or moved out of, a watched dir. */ + private const val DELETE_MASK = FileObserver.DELETE or FileObserver.MOVED_FROM + private const val EVENT_MASK = + FileObserver.CREATE or FileObserver.MODIFY or + FileObserver.MOVED_TO or FileObserver.CLOSE_WRITE or DELETE_MASK + } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/ProjectWatcher.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/ProjectWatcher.kt new file mode 100644 index 0000000000..c336d6e44d --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/ProjectWatcher.kt @@ -0,0 +1,28 @@ +package org.appdevforall.cotg.quickbuild.data + +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles + +/** + * Watches the open project on-device and reports coalesced batches of changed files. + * + * Triggers on file *change* from any source - the CoGo editor, a Termux script, a plugin + * write, a `git pull` - not on an editor save event, so edits made outside the editor still + * rebuild. Implementations run in CoGo's process on the phone; a Mac-side poller or an + * `adb`-driven trigger must never be wired into this path. + */ +interface ProjectWatcher { + /** + * Starts watching, invoking [onBatch] once per coalesced burst. + * + * Modified and created paths arrive in [ChangedFiles.Known.files], deleted ones in + * [ChangedFiles.Known.removed], with build intermediates and temp files already filtered + * out. Need not be idempotent; the session manager calls it once per live session. + * + * @param onBatch invoked once per coalesced burst; it runs on the implementation's own thread + * or scope, so it must not block. + */ + fun start(onBatch: (ChangedFiles.Known) -> Unit) + + /** Stop watching and release OS resources. Safe to call when not started. */ + fun stop() +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/ChangedFiles.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/ChangedFiles.kt new file mode 100644 index 0000000000..474f2d8d20 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/ChangedFiles.kt @@ -0,0 +1,71 @@ +package org.appdevforall.cotg.quickbuild.domain + +import java.io.File + +/** + * The set of files changed since the last successfully absorbed quick build. + * + * [Known] and [Unknown] are separate types because an empty [Known] set means "nothing + * changed" (a no-op save must not recompile), while [Unknown] means "we cannot tell" (crash + * recovery, missed watcher events) and makes the next build treat every source as dirty. + */ +sealed interface ChangedFiles { + /** + * Union of two changed-sets, reconciled per path with the newer batch winning. [other] is + * always the newer one, so modify-then-delete collapses to a removal and + * delete-then-recreate to a modification; a plain set union would leave the path in both + * sets and the executor would feed it to the daemon as changed AND removed. + * + * @param other the NEWER changed-set, whose verdict per path wins over this one's. + * @return the reconciled union, [Unknown] whenever either side is [Unknown] - a collapse that + * discards the enumerated side's paths, so a caller that routed on them must preserve the + * verdict itself (see `LiveReloadOrchestrator.stickyInvalidation`). + */ + operator fun plus(other: ChangedFiles): ChangedFiles + + /** True only for an empty [Known] set - [Unknown] is never empty. */ + val isEmpty: Boolean + + /** + * An enumerated changed-set. + * + * @property files paths modified or created since the last absorbed build. + * @property removed paths deleted since then, kept separate because a removal is classified by + * path shape alone (nothing is left on disk to inspect) and routes to the incremental + * compiler's removed-sources slot, which drops its outputs and recompiles dependents. + */ + data class Known( + val files: Set, + val removed: Set = emptySet(), + ) : ChangedFiles { + override fun plus(other: ChangedFiles): ChangedFiles = + when (other) { + is Known -> { + Known( + (files - other.removed) + other.files, + (removed - other.files) + other.removed, + ) + } + + Unknown -> { + Unknown + } + } + + override val isEmpty: Boolean + get() = files.isEmpty() && removed.isEmpty() + + companion object { + /** The shared "nothing changed" value; a no-op save must not recompile. */ + val EMPTY = Known(emptySet()) + } + } + + /** The changed-set could not be enumerated; every source counts as dirty. */ + data object Unknown : ChangedFiles { + override fun plus(other: ChangedFiles): ChangedFiles = Unknown + + override val isEmpty: Boolean + get() = false + } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationBaseline.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationBaseline.kt new file mode 100644 index 0000000000..06d8aa9c43 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationBaseline.kt @@ -0,0 +1,84 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import java.io.File + +/** + * The annotation-processor input the proxy app build ran against - the reference every later + * edit is compared to. + * + * Comparing against the baseline rather than the previous edit is what makes the fast path + * correct: the generated code in the installed proxy app came from this snapshot, so "unchanged + * versus the baseline" is exactly when that generated code is still right. + */ +class AnnotationBaseline private constructor( + /** + * Normalized absolute path -> the facts scanned at baseline. A null VALUE means the file was + * present but unscannable, which is why membership and value are asked separately. + */ + private val facts: Map, + /** + * Simple type names an annotated file reaches out to: supertypes, `@Database(entities = + * [...])` targets, `@Embedded` property types, converter classes. Declaring one of these + * forces a rebaseline, because such a file can change generated output without carrying an + * annotation itself - Room reads inherited fields and embedded classes. + */ + val anchorNames: Set, +) { + /** + * Facts recorded for [file] at baseline. + * + * @param file any path; matched after normalization, so relative and absolute forms agree. + * @return the recorded facts, or null both when the file was absent from the baseline and when + * it was present but unscannable - pair with [known] to tell those apart. + */ + fun factsFor(file: File): AnnotationFacts? = facts[key(file)] + + /** + * True when [file] existed in the baseline source set (scannable or not). + * + * @param file any path; matched after normalization, as in [factsFor]. + * @return true when the baseline scan saw the file, whatever the scan produced. + */ + fun known(file: File): Boolean = facts.containsKey(key(file)) + + companion object { + /** + * Scans the proxy app build's whole source set into a baseline. + * + * @param sources every source file the proxy app build compiled, since one missing here is + * later treated as newly added and so costs a rebaseline. + * @param profile which annotations count as processor input, and so which files + * contribute their referenced type names as anchors. + * @param readText content reader; returning null records the file as unscannable, + * which makes any later change to it rebaseline. + * @return the baseline every later edit is compared against. + */ + fun capture( + sources: List, + profile: AnnotationProcessorProfile, + readText: (File) -> String? = ::readOrNull, + ): AnnotationBaseline { + val facts = LinkedHashMap(sources.size) + val anchors = mutableSetOf() + for (source in sources) { + val scanned = readText(source)?.let(SourceAnnotationScanner::scan) + facts[key(source)] = scanned + if (scanned != null && scanned.annotations.any { profile.isProcessorInput(it, scanned) }) { + anchors += scanned.referencedTypeNames + } + } + return AnnotationBaseline(facts, anchors) + } + + /** + * Reads a source file's text, swallowing any I/O failure. + * + * @param file the source to read; decoded as UTF-8. + * @return the contents, or null when it cannot be read - which callers must treat as + * "deleted or unreadable", not as an empty file. + */ + fun readOrNull(file: File): String? = runCatching { file.readText() }.getOrNull() + + private fun key(file: File): String = file.absoluteFile.normalize().path + } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationFacts.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationFacts.kt new file mode 100644 index 0000000000..62bf0c0a45 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationFacts.kt @@ -0,0 +1,49 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +/** + * What a single source file tells us about annotation-processor input, extracted by + * [SourceAnnotationScanner]. Everything here is derived from text - the live reload path + * has no compiler front-end at classification time - so the scanner is deliberately + * over-inclusive and the analyzer treats "not sure" as "rebaseline". + * + * @property packageName declared package, empty for the default package. + * @property imports import FQNs as written; a star import keeps its trailing `.*`. + * @property annotations every `@Name(args)` occurrence in source order. + * @property declaredTypeNames simple names of types this file declares (class / + * interface / object / enum / record), including nested ones. + * @property declarationFingerprint the file's declaration surface - every code line outside a + * function/initializer body, comment- and whitespace-normalized - so two revisions sharing one + * differ only inside executable bodies. + * @property referencedTypeNames capitalized identifiers appearing in the declaration + * surface (types a processor could follow out of this file, e.g. an `@Embedded` + * property's class or a `@Database(entities = [...])` argument). + */ +data class AnnotationFacts( + val packageName: String, + val imports: List, + val annotations: List, + val declaredTypeNames: Set, + val declarationFingerprint: List, + val referencedTypeNames: Set, +) + +/** + * One annotation occurrence, exactly as written. + * + * @property name the name at the use site - simple (`Entity`) or qualified + * (`androidx.room.Entity`), without any use-site target. + * @property arguments the parenthesized argument text with whitespace collapsed, empty when there + * is no argument list, and itself processor input - Room reads `@Query("...")`'s SQL and + * `@ColumnInfo(name = ...)`'s column name. + * @property useSiteTarget the Kotlin use-site target (`field` in `@field:Json`), kept separate + * from [name] so imports still resolve the name but part of equality, because `@get:Json` and + * `@field:Json` are different processor input. + */ +data class AnnotationUse( + val name: String, + val arguments: String, + val useSiteTarget: String = "", +) { + /** Last dot-segment of [name] - what an import has to match to resolve it. */ + val simpleName: String get() = name.substringAfterLast('.') +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt new file mode 100644 index 0000000000..0972de3070 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt @@ -0,0 +1,152 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import org.slf4j.LoggerFactory +import java.io.File + +/** + * Decides whether a code change can have moved annotation-processor output, and so whether + * the quick path must give way to a full Gradle rebaseline. + * + * Without it, a project with any processor configured would have to rebaseline on every edit, + * because a stale generated class is indistinguishable from a fresh one at run time; with it, + * only edits that touch processor input pay that cost. + */ +interface AnnotationImpact { + /** True when the project configures at least one annotation processor. */ + val active: Boolean + + /** + * Checks a build's changed code files against the processor input. + * + * @param changedCodeFiles the `.kt`/`.java` paths this build would compile, deletions + * included; other file kinds are the classifier's business, not this one's. + * @return a human-readable reason to rebaseline - the FIRST file that forces one, not all + * of them - or null when every changed file is provably outside processor input. + */ + fun escalation(changedCodeFiles: List): String? + + /** No processors configured: nothing to protect, nothing ever escalates. */ + object Inactive : AnnotationImpact { + override val active: Boolean = false + + override fun escalation(changedCodeFiles: List): String? = null + } +} + +/** + * An [AnnotationImpact] whose delegate can be swapped, so a rebaseline can move the reference + * point without rebuilding the orchestrator. + * + * The Gradle build that just ran is the new baseline; comparing later edits against the + * pre-rebaseline snapshot would keep charging for changes it already absorbed. + * + * @property delegate the analyzer in force now; every call reads it, so a swap takes effect on + * the next classification with no re-wiring. + */ +class SwitchableAnnotationImpact( + var delegate: AnnotationImpact, +) : AnnotationImpact { + override val active: Boolean get() = delegate.active + + override fun escalation(changedCodeFiles: List): String? = delegate.escalation(changedCodeFiles) +} + +/** + * The real [AnnotationImpact]: compares each changed file against the proxy app build's + * [AnnotationBaseline], rebaselining when a processor-relevant file changes its annotations or + * declaration surface, is added or deleted, declares an [AnnotationBaseline.anchorNames] type, or + * cannot be scanned. Edits confined to function or initializer bodies stay on the live reload + * path: every processor the profile knows generates from declarations and annotation arguments, + * not statement bodies. + * + * @param profile which annotations this project's processors consume; an unrecognized processor + * widens that to nearly everything. + * @param baseline the proxy app build's snapshot, and so the fixed reference point until the + * next rebaseline replaces this analyzer. + * @param readText reader for a changed file's CURRENT text; null means deleted or unreadable, + * which is a rebaseline whenever the file fed a processor. + */ +class AnnotationImpactAnalyzer( + private val profile: AnnotationProcessorProfile, + private val baseline: AnnotationBaseline, + private val readText: (File) -> String? = AnnotationBaseline::readOrNull, +) : AnnotationImpact { + private val log = LoggerFactory.getLogger("QB-AnnotationImpact") + + override val active: Boolean get() = profile.hasProcessors + + override fun escalation(changedCodeFiles: List): String? { + if (!active) return null + for (file in changedCodeFiles) { + val reason = escalationFor(file) + if (reason != null) { + log.info("Quick build: annotation-processor input changed in {} ({})", file.name, reason) + return "${file.name}: $reason" + } + } + return null + } + + /** + * Why [file] forces a rebaseline, or null when it provably misses processor input. + * + * @param file one changed code file, compared against its baseline facts; it need not still + * exist, since a deletion is itself an escalation once the file fed a processor. + * @return a short human-readable cause for the user-facing message, or null to keep the + * file on the live reload path. + */ + private fun escalationFor(file: File): String? { + val old = baseline.factsFor(file) + val existedAtBaseline = baseline.known(file) + val current = readText(file) + val new = current?.let(SourceAnnotationScanner::scan) + + if (existedAtBaseline && old == null) { + return "baseline copy could not be scanned" + } + if (current == null) { + // Deleted (or unreadable). Only matters if it fed a processor directly or as an + // anchor; a deleted plain file cannot change generated output. + if (old == null) return null + if (old.hasProcessorInput()) return "annotated file was deleted" + val anchors = old.declaredTypeNames.intersect(baseline.anchorNames) + return if (anchors.isEmpty()) { + null + } else { + "deleted ${anchors.sorted().joinToString()}, read by an annotated declaration" + } + } + if (new == null) { + return "file could not be scanned" + } + + val oldIsInput = old?.hasProcessorInput() == true + val newIsInput = new.hasProcessorInput() + if (oldIsInput || newIsInput) { + if (old == null) return "new file declares processor-relevant annotations" + if (!oldIsInput || !newIsInput) return "processor-relevant annotations added or removed" + if (old.processorAnnotations() != new.processorAnnotations()) { + return "processor-relevant annotations changed" + } + if (old.declarationFingerprint != new.declarationFingerprint) { + return "declarations of an annotated file changed" + } + return null + } + + // A plain file only matters when it actually moved AND declares a type an annotated + // declaration reads (an entity base class, an `@Embedded` value type, a converter). + // A watcher event on an untouched file must not cost a rebaseline. + if (old != null && old.declarationFingerprint == new.declarationFingerprint) return null + val declaredAnchors = + (new.declaredTypeNames + old?.declaredTypeNames.orEmpty()).intersect(baseline.anchorNames) + if (declaredAnchors.isNotEmpty()) { + return "declares ${declaredAnchors.sorted().joinToString()}, read by an annotated declaration" + } + return null + } + + private fun AnnotationFacts.hasProcessorInput(): Boolean = annotations.any { profile.isProcessorInput(it, this) } + + private fun AnnotationFacts.processorAnnotations(): List = annotations.filter { profile.isProcessorInput(it, this) } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfile.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfile.kt new file mode 100644 index 0000000000..89d373b235 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfile.kt @@ -0,0 +1,270 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +/** + * Which annotations count as processor input for this project, derived from the processors the + * proxy app build reported (setup.json `annotationProcessors`). + * + * Two modes, because being permissive here ships stale generated code: with every processor + * recognized, only annotations from those processors' own packages are input; with any processor + * unrecognized, every annotation is input except the language-level ones ([LANGUAGE_INERT]). + */ +class AnnotationProcessorProfile private constructor( + /** Dependency coordinates as reported by the proxy app build; empty means no processors. */ + val processorCoordinates: List, + /** Vocabulary of the recognized processors only, deduplicated by [ProcessorSpec.id]. */ + private val specs: List, + /** + * True when at least one coordinate matched no known processor, which switches the profile + * into the conservative mode where every non-inert annotation counts as input. + */ + private val hasUnrecognized: Boolean, +) { + /** False when the project configures no annotation processor at all. */ + val hasProcessors: Boolean get() = processorCoordinates.isNotEmpty() + + private val packages: Set = specs.flatMapTo(mutableSetOf()) { it.annotationPackages } + private val simpleNames: Set = specs.flatMapTo(mutableSetOf()) { it.annotationSimpleNames } + + /** + * True when [use], as written in [facts], can feed a configured processor. + * + * @param use one annotation occurrence, name exactly as the source wrote it. + * @param facts the same file's facts, needed for the imports that resolve a simple name. + * @return true when the annotation could be processor input, deliberately over-inclusive on an + * unresolvable name because a wrong `false` here would ship stale generated code. + */ + fun isProcessorInput( + use: AnnotationUse, + facts: AnnotationFacts, + ): Boolean { + if (!hasProcessors) return false + val resolved = resolve(use, facts) + if (resolved != null) { + if (isLanguageInert(resolved)) return false + if (hasUnrecognized) return true + return packages.any { resolved.startsWith("$it.") } + } + // Unresolvable (star import, same-package annotation, missing import): the simple name + // is all we have. Known processor vocabulary wins; otherwise only unrecognized mode + // treats it as input, minus the stdlib names that are in scope without an import. + if (use.simpleName in LANGUAGE_INERT_NAMES) return false + return use.simpleName in simpleNames || hasUnrecognized + } + + /** + * FQN of [use] if imports (or the use site itself) pin it down. + * + * @param use one annotation occurrence; already fully qualified at the use site when its + * name carries a dot. + * @param facts the same file's facts, read only for its import list. + * @return the resolved FQN, or null when nothing pins the simple name down (star import, + * same-package annotation, missing import) and the caller must fall back to that name. + */ + private fun resolve( + use: AnnotationUse, + facts: AnnotationFacts, + ): String? { + if (use.name.contains('.')) return use.name + val simple = use.simpleName + facts.imports.firstOrNull { it.substringAfterLast('.') == simple }?.let { return it } + return null + } + + private fun isLanguageInert(fqn: String): Boolean = LANGUAGE_INERT.any { fqn.startsWith("$it.") } + + /** One processor's annotation vocabulary. */ + data class ProcessorSpec( + /** Stable key for the processor; two coordinates mapping to it contribute one spec. */ + val id: String, + /** Packages whose annotations this processor consumes; matched as an FQN prefix. */ + val annotationPackages: Set, + /** + * Names this processor consumes, used when an import cannot resolve the use site and so + * not exhaustive by design - it is a fallback on top of the package match. + */ + val annotationSimpleNames: Set, + ) + + companion object { + /** No processors configured: nothing is processor input, nothing ever escalates. */ + val NONE = AnnotationProcessorProfile(emptyList(), emptyList(), hasUnrecognized = false) + + /** + * Builds the profile for a project's configured processors. + * + * @param coordinates processor dependency coordinates (`group:artifact:version`, or + * whatever the proxy app build could report - matching is substring-based, so a + * version-catalog alias like `libs.room.compiler` still identifies Room). + * @return [NONE] for an empty or blank-only list; otherwise a profile that turns + * conservative as soon as a single coordinate goes unrecognized. + */ + fun of(coordinates: List): AnnotationProcessorProfile { + val cleaned = coordinates.map { it.trim() }.filter { it.isNotEmpty() } + if (cleaned.isEmpty()) return NONE + val specs = mutableListOf() + var unrecognized = false + for (coordinate in cleaned) { + val spec = KNOWN.firstOrNull { (marker, _) -> coordinate.contains(marker, ignoreCase = true) } + if (spec == null) unrecognized = true else specs += spec.second + } + return AnnotationProcessorProfile(cleaned, specs.distinctBy { it.id }, unrecognized) + } + + private val ROOM = + ProcessorSpec( + id = "room", + annotationPackages = setOf("androidx.room"), + annotationSimpleNames = + setOf( + "Database", + "Entity", + "Dao", + "Query", + "Insert", + "Update", + "Delete", + "Upsert", + "PrimaryKey", + "ColumnInfo", + "Embedded", + "Relation", + "Ignore", + "Index", + "ForeignKey", + "TypeConverter", + "TypeConverters", + "Transaction", + "RawQuery", + "RewriteQueriesToDropUnusedColumns", + "DatabaseView", + "Fts3", + "Fts4", + "AutoMigration", + "DeleteColumn", + "DeleteTable", + "RenameColumn", + "RenameTable", + "MapInfo", + "SkipQueryVerification", + "Junction", + ), + ) + + private val DAGGER_HILT = + ProcessorSpec( + id = "dagger-hilt", + annotationPackages = + setOf("dagger", "javax.inject", "jakarta.inject", "androidx.hilt", "dagger.hilt"), + annotationSimpleNames = + setOf( + "Inject", + "Module", + "Provides", + "Binds", + "Component", + "Subcomponent", + "AndroidEntryPoint", + "HiltAndroidApp", + "HiltViewModel", + "HiltWorker", + "InstallIn", + "EntryPoint", + "Qualifier", + "Scope", + "Singleton", + "Named", + "IntoSet", + "IntoMap", + "BindsInstance", + "Assisted", + "AssistedInject", + "AssistedFactory", + "MapKey", + "Reusable", + "DefineComponent", + ), + ) + + private val MOSHI = + ProcessorSpec( + id = "moshi", + annotationPackages = setOf("com.squareup.moshi"), + annotationSimpleNames = setOf("JsonClass", "Json", "JsonQualifier"), + ) + + private val GLIDE = + ProcessorSpec( + id = "glide", + annotationPackages = setOf("com.bumptech.glide.annotation"), + annotationSimpleNames = setOf("GlideModule", "GlideExtension", "GlideOption", "GlideType"), + ) + + private val AUTO_VALUE = + ProcessorSpec( + id = "auto-value", + annotationPackages = setOf("com.google.auto.value", "com.google.auto.service"), + annotationSimpleNames = setOf("AutoValue", "AutoService", "Memoized", "CopyAnnotations"), + ) + + /** + * Coordinate marker -> vocabulary, matched as a substring. A coordinate matching + * nothing here flips the profile into the conservative unrecognized mode. + */ + private val KNOWN: List> = + listOf( + "room" to ROOM, + "hilt" to DAGGER_HILT, + "dagger" to DAGGER_HILT, + "moshi" to MOSHI, + "glide" to GLIDE, + "auto-value" to AUTO_VALUE, + "auto.value" to AUTO_VALUE, + "auto-service" to AUTO_VALUE, + "auto.service" to AUTO_VALUE, + ) + + /** + * Packages whose annotations are language/compiler-level and cannot be a + * processor's input, so they never force a rebaseline even in unrecognized mode. + * Kept deliberately narrow: `androidx.annotation` and `androidx.compose` are NOT + * here, because third-party processors (Showkase, Compose Destinations and kin) + * really do read Compose annotations. + */ + private val LANGUAGE_INERT = + setOf("kotlin", "java.lang", "org.jetbrains.annotations") + + /** + * The [LANGUAGE_INERT] annotations that are in scope with no import, so a use site + * cannot be resolved to a package. Treating a same-package user annotation with one + * of these names as inert is the one accepted (and vanishingly rare) blind spot. + */ + private val LANGUAGE_INERT_NAMES = + setOf( + "Deprecated", + "Suppress", + "SuppressWarnings", + "Override", + "SafeVarargs", + "FunctionalInterface", + "Throws", + "OptIn", + "RequiresOptIn", + "PublishedApi", + "JvmStatic", + "JvmField", + "JvmName", + "JvmOverloads", + "JvmSynthetic", + "JvmInline", + "Synchronized", + "Volatile", + "Transient", + "Strictfp", + "DslMarker", + "Target", + "Retention", + "MustBeDocumented", + "Repeatable", + ) + } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/README.md b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/README.md new file mode 100644 index 0000000000..7c35f74f29 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/README.md @@ -0,0 +1,11 @@ +# `domain/annotations/` - does a change feed an annotation processor + +Decides whether a code edit could have moved annotation-processor (KSP/kapt) output, so the classifier knows when the live reload path must give way to a full Gradle rebaseline. Compares each changed file against a baseline captured from the proxy app build, using a text-only scan (no compiler front-end) that is deliberately over-inclusive: "not sure" means rebaseline. Pure JVM; no Android. + +| File | Purpose | +| --- | --- | +| [`AnnotationImpact.kt`](AnnotationImpact.kt) | The `AnnotationImpact` interface (plus `Inactive`, `SwitchableAnnotationImpact`, and the real `AnnotationImpactAnalyzer`) that maps changed code files to a rebaseline reason or null. | +| [`AnnotationBaseline.kt`](AnnotationBaseline.kt) | The proxy app build's scanned source set (per-file facts plus anchor type names) that every later edit is compared against. | +| [`AnnotationProcessorProfile.kt`](AnnotationProcessorProfile.kt) | Which annotations count as processor input, derived from the reported processor coordinates; recognizes Room/Hilt/Moshi/Glide/AutoValue, turns conservative on any unrecognized processor. | +| [`SourceAnnotationScanner.kt`](SourceAnnotationScanner.kt) | Extracts `AnnotationFacts` from Kotlin/Java text without a parser; strips comments, masks string literals, fingerprints the declaration surface, excludes function bodies. | +| [`AnnotationFacts.kt`](AnnotationFacts.kt) | The value types the scanner produces: `AnnotationFacts` (package, imports, annotations, declared/referenced type names, declaration fingerprint) and `AnnotationUse`. | diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt new file mode 100644 index 0000000000..5cd389c120 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt @@ -0,0 +1,377 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +/** + * Extracts [AnnotationFacts] from Kotlin/Java source text without a compiler front-end. + * + * Text rather than a parser because classification runs on every save, before any compile, with no + * resolved PSI. It aims at never missing a change that could alter processor output and pays for + * that with over-inclusiveness: string literals are kept verbatim (an `@Query("SELECT ...")` edit + * IS processor input), only an unambiguous function/initializer body leaves the declaration + * fingerprint, and a structural surprise returns null, which the analyzer reads as "rebaseline". + * + * @see AnnotationImpactAnalyzer for how the facts turn into a routing decision. + */ +object SourceAnnotationScanner { + /** Placeholder standing in for string/char-literal content while scanning structure. */ + private const val MASKED = '\u0001' + + /** + * Extracts one file's facts from its source text. + * + * @param text the whole file, Kotlin or Java; the language is inferred from what the text + * contains rather than from any extension. + * @return the facts, or null when the text could not be scanned confidently (unbalanced braces, + * unterminated comment or raw string), which callers read as "assume processor input + * changed". + */ + fun scan(text: String): AnnotationFacts? { + val prepared = prepare(text) ?: return null + val bodyMask = markFunctionBodies(prepared) ?: return null + + val fingerprint = + prepared.codeLines + .filterIndexed { index, _ -> !bodyMask[index] } + .map { it.normalizeWhitespace() } + .filter { it.isNotEmpty() } + + val annotations = extractAnnotations(prepared) + val imports = + prepared.codeLines.mapNotNull { line -> + IMPORT.find(line.trim())?.groupValues?.get(1) + } + val packageName = + prepared.codeLines.firstNotNullOfOrNull { line -> + PACKAGE.find(line.trim())?.groupValues?.get(1) + } ?: "" + + val declared = mutableSetOf() + for (line in prepared.codeLines) { + TYPE_DECLARATION.findAll(line).forEach { declared += it.groupValues[2] } + } + + val referenced = mutableSetOf() + fingerprint.forEach { line -> CAPITALIZED.findAll(line).forEach { referenced += it.value } } + annotations.forEach { use -> + CAPITALIZED.findAll(use.arguments).forEach { referenced += it.value } + } + + return AnnotationFacts( + packageName = packageName, + imports = imports, + annotations = annotations, + declaredTypeNames = declared, + declarationFingerprint = fingerprint, + referencedTypeNames = referenced, + ) + } + + /** + * Comment-stripped source plus a structure mask. + * + * @property codeLines source lines with comments removed and string literals intact. + * @property maskLines the same lines with every string/char-literal character replaced + * by [MASKED], so brace counting and `@` detection never fire inside a literal. + */ + private class Prepared( + val codeLines: List, + val maskLines: List, + ) + + /** + * Strips comments and builds the literal mask. + * + * @param text one source file's whole contents, Kotlin or Java. + * @return the code/mask line pair, or null when a block comment or a literal never closes - + * a half-typed file the caller must not draw conclusions from. + */ + private fun prepare(text: String): Prepared? { + val code = StringBuilder() + val mask = StringBuilder() + var i = 0 + var state = State.CODE + var quote = ' ' + val n = text.length + while (i < n) { + val c = text[i] + val next = if (i + 1 < n) text[i + 1] else '\u0000' + when (state) { + State.CODE -> { + when { + c == '/' && next == '/' -> { + while (i < n && text[i] != '\n') i++ + continue + } + + c == '/' && next == '*' -> { + state = State.BLOCK_COMMENT + i += 2 + continue + } + + c == '"' && next == '"' && i + 2 < n && text[i + 2] == '"' -> { + state = State.RAW_STRING + code.append("\"\"\"") + mask.append("\"\"\"") + i += 3 + continue + } + + c == '"' || c == '\'' -> { + state = State.STRING + quote = c + code.append(c) + mask.append(c) + i++ + continue + } + + else -> { + code.append(c) + mask.append(c) + i++ + } + } + } + + State.BLOCK_COMMENT -> { + // Newlines survive so line numbering (and thus the fingerprint's line + // structure) is not disturbed by a multi-line comment. + if (c == '\n') { + code.append('\n') + mask.append('\n') + } + if (c == '*' && next == '/') { + state = State.CODE + i += 2 + continue + } + i++ + } + + State.STRING -> { + code.append(c) + mask.append(if (c == '\n') '\n' else MASKED) + when { + // A line break inside a single-quoted literal means the literal was + // never closed: bail rather than guess where it ended. + c == '\n' -> { + return null + } + + c == '\\' && i + 1 < n -> { + code.append(text[i + 1]) + mask.append(MASKED) + i += 2 + continue + } + + c == quote -> { + state = State.CODE + } + } + i++ + } + + State.RAW_STRING -> { + if (c == '"' && next == '"' && i + 2 < n && text[i + 2] == '"') { + state = State.CODE + code.append("\"\"\"") + mask.append("\"\"\"") + i += 3 + continue + } + code.append(c) + mask.append(if (c == '\n') '\n' else MASKED) + i++ + } + } + } + if (state != State.CODE) return null + val codeLines = code.toString().lines() + val maskLines = mask.toString().lines() + // Every index into the mask is a line index, so the two line counts must agree. A + // backslash-escaped line break inside a single-quoted literal appends the newline to + // `code` but MASKED to `mask`, and scanning past that indexes the mask out of bounds - + // which, on this call path, is an uncaught throw in a handler-less scope. Illegal in both + // languages, so bail like any other structural surprise rather than guess. + if (codeLines.size != maskLines.size) return null + return Prepared(codeLines, maskLines) + } + + private enum class State { CODE, BLOCK_COMMENT, STRING, RAW_STRING } + + /** + * Flags each line that sits inside a function or initializer body. + * + * Conservative: a line counts as body only when its opening line matched + * [FUNCTION_SIGNATURE] and contributed exactly one net brace. Class bodies, `when` blocks, + * property-initializer lambdas and multi-line signatures all stay in the fingerprint. + * + * @param prepared the file to walk; brace counting runs over its mask lines, and the + * signature match over the code lines at the same indexes. + * @return one flag per line, or null when brace nesting does not balance - the caller must + * not trust the file. + */ + private fun markFunctionBodies(prepared: Prepared): BooleanArray? { + val result = BooleanArray(prepared.maskLines.size) + var depth = 0 + var bodyDepth = -1 + for ((index, masked) in prepared.maskLines.withIndex()) { + val inBody = bodyDepth >= 0 + result[index] = inBody + val opens = masked.count { it == '{' } + val closes = masked.count { it == '}' } + val opensFunction = + !inBody && opens - closes == 1 && FUNCTION_SIGNATURE.containsMatchIn(prepared.codeLines[index]) + var seenOpen = 0 + for (c in masked) { + when (c) { + '{' -> { + depth++ + seenOpen++ + if (opensFunction && seenOpen == opens) bodyDepth = depth + } + + '}' -> { + if (bodyDepth == depth) bodyDepth = -1 + depth-- + if (depth < 0) return null + } + } + } + } + return if (depth == 0) result else null + } + + /** + * Collects every `@Name(...)` in the file, in source order. + * + * A Kotlin use-site target is split into [AnnotationUse.useSiteTarget] so imports still + * resolve the bare name while `@get:Json` and `@field:Json` stay distinct. Argument text + * keeps literals verbatim and collapses whitespace: reformatting is a no-op, a value edit + * is not. + * + * @param prepared the file to walk; `@` and parens are found on the mask lines while the + * recorded name and argument text are cut from the code lines at the same offsets. + * @return the uses in source order, truncated at the first unbalanced argument list rather + * than dropped, so a half-typed annotation does not hide the ones above it. + */ + private fun extractAnnotations(prepared: Prepared): List { + val mask = prepared.maskLines.joinToString("\n") + val code = prepared.codeLines.joinToString("\n") + val result = mutableListOf() + var i = 0 + while (i < mask.length) { + if (mask[i] != '@') { + i++ + continue + } + // `@` inside an identifier is not an annotation (Kotlin `a@b` labels, emails + // inside masked strings can't reach here). + if (i > 0 && (mask[i - 1].isLetterOrDigit() || mask[i - 1] == '_' || mask[i - 1] == '@')) { + i++ + continue + } + var j = i + 1 + // Optional Kotlin use-site target, e.g. `@field:Json`. + var target = "" + val targetEnd = readIdentifierPath(mask, j) + if (targetEnd > j && targetEnd < mask.length && mask[targetEnd] == ':') { + target = code.substring(j, targetEnd) + j = targetEnd + 1 + } + val nameEnd = readIdentifierPath(mask, j) + if (nameEnd == j) { + i++ + continue + } + val name = code.substring(j, nameEnd) + var k = nameEnd + while (k < mask.length && (mask[k] == ' ' || mask[k] == '\t')) k++ + var arguments = "" + if (k < mask.length && mask[k] == '(') { + val close = matchParen(mask, k) ?: return result + arguments = code.substring(k, close + 1).normalizeWhitespace() + k = close + 1 + } + result += AnnotationUse(name = name, arguments = arguments, useSiteTarget = target) + i = k + } + return result + } + + /** + * End index (exclusive) of a dotted identifier starting at [start]. + * + * @param text masked source, so characters inside a literal cannot pass as an identifier. + * @param start index the identifier must begin at; no leading whitespace is skipped. + * @return one past the identifier's last character, or [start] when none begins there; a + * trailing dot is left to the next token. + */ + private fun readIdentifierPath( + text: String, + start: Int, + ): Int { + var i = start + if (i >= text.length || !(text[i].isLetter() || text[i] == '_')) return start + while (i < text.length && (text[i].isLetterOrDigit() || text[i] == '_' || text[i] == '.')) i++ + // A trailing dot belongs to the next token, not the name. + while (i > start && text[i - 1] == '.') i-- + return i + } + + /** + * Index of the `)` closing the `(` at [open]. + * + * @param text masked source, so parentheses inside a string literal are not counted. + * @param open index of the opening `(`; depth is counted from that character onward. + * @return the matching `)` index, or null when the parentheses never balance before the + * end of the file. + */ + private fun matchParen( + text: String, + open: Int, + ): Int? { + var depth = 0 + var i = open + while (i < text.length) { + when (text[i]) { + '(' -> { + depth++ + } + + ')' -> { + depth-- + if (depth == 0) return i + } + } + i++ + } + return null + } + + private fun String.normalizeWhitespace(): String = trim().replace(WHITESPACE, " ") + + private val WHITESPACE = Regex("\\s+") + private val IMPORT = Regex("^import\\s+(?:static\\s+)?([\\w.]+(?:\\.\\*)?)") + private val PACKAGE = Regex("^package\\s+([\\w.]+)") + private val TYPE_DECLARATION = + Regex("\\b(class|interface|object|enum|record|@interface)\\s+([A-Za-z_][A-Za-z0-9_]*)") + private val CAPITALIZED = Regex("\\b[A-Z][A-Za-z0-9_]*\\b") + + /** + * Lines that unambiguously open a function/initializer body. Kotlin: `fun`, `init`, + * a property accessor, a secondary `constructor`. Java: a method signature - a name + + * parameter list + `{` with no statement/type keyword in front of it (which would make + * it an `if`/`for`/`class`/... block instead). + */ + private val FUNCTION_SIGNATURE = + Regex( + "(\\bfun\\s)" + + "|(\\binit\\s*\\{)" + + "|(\\b(get|set)\\s*\\()" + + "|(\\bconstructor\\s*\\()" + + "|(^\\s*(?!.*\\b(class|interface|enum|record|new|if|for|while|when|switch|catch|do|else|try|synchronized)\\b)" + + "[\\w<>\\[\\],.\\s@]*\\b\\w+\\s*\\([^;]*\\)\\s*(throws[\\w.,\\s]+)?\\{\\s*$)", + ) +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRoute.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRoute.kt new file mode 100644 index 0000000000..4a11bd6737 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRoute.kt @@ -0,0 +1,131 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +/** + * Which build path a coalesced changed-set takes - the cheapest one that is still correct. + * + * Anything the live reload path cannot absorb with certainty routes to [FullGradleBuild], so + * the proxy app never runs stale code. + */ +sealed interface BuildRoute { + /** + * The session baseline is stale; only a real Gradle build can absorb this change. + * + * @property reason what invalidated the baseline; the session manager reports it to the user + * and decides whether the fallback rebuilds the proxy app. + */ + data class FullGradleBuild( + val reason: InvalidationReason, + ) : BuildRoute + + /** Resources changed, no code: aapt2 relink, reuse cached dex. */ + data object ResourcesOnly : BuildRoute + + /** + * assets/ only: no compile, no relink - deploy the changed asset bytes. + * + * Changed assets are included in every route's deploy payload; this route only means the + * payload carries nothing else. + */ + data object AssetsOnly : BuildRoute + + /** Code changed, no resources: incremental compile, then d8 over the whole class tree. */ + data object CodeOnly : BuildRoute + + /** Mixed save: relink AND compile - never serve stale resources beside new code. */ + data object CodeAndResources : BuildRoute + + /** Empty known changed-set: nothing to rebuild (a forced tap may still redeploy). */ + data object NoOp : BuildRoute + + /** + * Background warm-up right after provisioning: compile + dex the whole module once so the + * daemon pays the compiler warm-up before the user's first save instead of on it. + * + * Deploys nothing - the proxy app already runs exactly these sources. Never produced by + * the classifier; only [org.appdevforall.cotg.quickbuild.domain.reload.LiveReloadOrchestrator.onWarmCompileRequested] constructs it. + */ + data object WarmCompile : BuildRoute +} + +/** + * Whether this route recompiles user code, so a deploy from it can have replaced classes a + * live service or provider still holds ([org.appdevforall.cotg.quickbuild.domain.session.QuickBuildNotice.STALE_COMPONENT_HELPERS]). + * + * A forced [BuildRoute.NoOp] recompiles the whole module, so it counts. Says nothing about + * whether anything DEPLOYED - a [BuildRoute.WarmCompile] recompiles everything and deploys + * nothing, so a caller reasoning about the running app has to exclude it separately. + */ +val BuildRoute.recompilesCode: Boolean + get() = + when (this) { + BuildRoute.CodeOnly, + BuildRoute.CodeAndResources, + BuildRoute.NoOp, + BuildRoute.WarmCompile, + -> true + + BuildRoute.ResourcesOnly, + BuildRoute.AssetsOnly, + is BuildRoute.FullGradleBuild, + -> false + } + +/** Why a quick-build session baseline can no longer absorb edits on the live reload path. */ +enum class InvalidationReason { + /** `AndroidManifest.xml` changed: components, permissions and the proxy transform all move. */ + MANIFEST_CHANGED, + + /** A Gradle build script, properties file or version catalog changed: the classpath may move. */ + GRADLE_CONFIG_CHANGED, + + /** + * A watched file changed whose packaging semantics the live reload path does not implement + * (e.g. a java-resource under src/), or cannot deliver on this device - an asset below API + * 30, where the runtime has no `ResourcesLoader` to serve the deployed payload from. + */ + UNSUPPORTED_FILE_CHANGED, + + /** + * A code/resource/asset file changed in another Gradle module. The live reload path + * compiles only the app module against a frozen dependency classpath - other modules' + * output is baked into the baseline - so a library-module edit needs a full build. + */ + NON_APP_MODULE_SOURCE_CHANGED, + + /** A full Gradle build ran outside the session and moved the baseline. */ + EXTERNAL_FULL_BUILD, + + /** + * A changed source could have moved annotation-processor (KSP/kapt) output - a Room + * entity, a `@Query`, a Hilt module. Only a real Gradle build re-runs the processor. See + * `domain/annotations/AnnotationImpact.kt` for which edits provably miss processor input + * and so stay on the live reload path. + */ + ANNOTATION_PROCESSOR_INPUT_CHANGED, + + /** + * The installed baseline predates the component-restart contract (setup.json schema < 2), + * so its runtime would hot-swap a restart-requiring deploy and leave a live + * service/provider stale. Rebaselining regenerates setup.json and reinstalls. + */ + OUTDATED_BASELINE, + + /** + * The same infrastructure failure twice running, for something no edit can clear (a relink + * that cannot resolve the baseline's library resource snapshot), so only a fresh baseline can + * absorb the pending set. Reported at most once per baseline, so a rebuild that fails leaves + * plain build failures. Never raised for a compile error - see + * `LiveReloadOrchestrator.recordFailureLocked`. + */ + RELOAD_PIPELINE_FAILED, + + /** + * A proxy app rebuild produced a good APK but the OS install prompt was never confirmed. + * + * The prompt may never even appear: Android defers the PENDING_USER_ACTION broadcast until + * CoGo is foregrounded, and the dialog-owning subscriber is EventBus lifecycle-bound, so the + * deferred delivery can land before it re-registers. The next Quick Build tap or return to + * the foreground re-runs the rebuild and re-prompts. + */ + INSTALL_NOT_CONFIRMED, +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifier.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifier.kt new file mode 100644 index 0000000000..616d037b73 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifier.kt @@ -0,0 +1,300 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.annotations.AnnotationImpact +import java.io.File + +/** + * Picks the cheapest correct [BuildRoute] for a coalesced changed-set. + * + * Classification is by path shape, not file content: Gradle build files and + * `AndroidManifest.xml` invalidate the session, `src//res` and `.../assets` hold + * resources and assets, `.kt`/`.java` are code, and anything else under `src/` routes to Gradle + * because the live reload path does not implement its packaging. + * + * @param annotationImpact the one content-aware step: with a KSP/kapt processor configured, a + * changed source that could have moved generated code escalates to a Gradle rebaseline + * ([AnnotationImpact.Inactive] leaves a processor-free project unaffected). + * @param fastPathRoots the app module's live-reload source scope - the quick path compiles only + * that module against a frozen dependency classpath, so a change elsewhere routes to + * [InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED], and an empty list disables the boundary. + * @param assetsLiveReloadable whether this device can serve a deployed asset payload - the + * runtime's asset overlay rides the API 30+ `ResourcesLoader`, so false routes any + * asset-bearing set to Gradle rather than acking a reload the app cannot see. + */ +class ChangeClassifier( + private val annotationImpact: AnnotationImpact = AnnotationImpact.Inactive, + private val fastPathRoots: List = emptyList(), + private val assetsLiveReloadable: Boolean = true, +) { + /** + * Routes one coalesced changed-set. + * + * [ChangedFiles.Unknown] recompiles everything ON the quick path + * ([BuildRoute.CodeAndResources]), not as a Gradle fallback - unless a processor is + * configured, since an unenumerable change cannot be proven to miss processor input. + * + * @param changes the coalesced changed-set for one build; modified and removed paths are + * classified alike, by shape. + * @return the cheapest correct route, which is [BuildRoute.FullGradleBuild] as soon as any + * single path in the set demands it - the verdict is not per file. + */ + fun classify(changes: ChangedFiles): BuildRoute { + val known = + when (changes) { + ChangedFiles.Unknown -> { + return if (annotationImpact.active) { + BuildRoute.FullGradleBuild(InvalidationReason.ANNOTATION_PROCESSOR_INPUT_CHANGED) + } else { + BuildRoute.CodeAndResources + } + } + + is ChangedFiles.Known -> { + changes + } + } + + if (known.isEmpty) { + return BuildRoute.NoOp + } + + var hasResources = false + var hasAssets = false + val codeFiles = mutableListOf() + + // A removed file classifies by the same path shape as a modified one - its role is + // still legible from its extension even though the file is gone. Removals with no + // recognized shape are dropped upstream (QuickBuildSessionManager.onWatcherBatch). + for (file in known.files + known.removed) { + val kind = kindOf(file) + if (kind == FileKind.CODE || kind == FileKind.RESOURCE || kind == FileKind.ASSET) { + if (fastPathRoots.isNotEmpty() && fastPathRoots.none { isUnder(file, it) }) { + return BuildRoute.FullGradleBuild(InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED) + } + // Only src/main is on the quick path: allSources, resDirs and assetRoots all read + // src/main alone, while the watch scope is the whole src tree. A debug or flavor + // source set would otherwise compile and relink nothing while the deploy claimed a + // reload the running app cannot show. Those DO ship in the built variant, so a + // full build is the honest answer; test source sets do not, and TestSourceFilter + // drops them upstream rather than spending a build on an app that cannot differ. + val sourceSet = sourceSetName(file) + if (sourceSet != null && sourceSet != "main") { + return BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED) + } + } + when (kind) { + FileKind.GRADLE_CONFIG -> { + return BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED) + } + + FileKind.MANIFEST -> { + return BuildRoute.FullGradleBuild(InvalidationReason.MANIFEST_CHANGED) + } + + FileKind.UNSUPPORTED -> { + return BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED) + } + + FileKind.CODE -> { + codeFiles += file + } + + FileKind.RESOURCE -> { + hasResources = true + } + + FileKind.ASSET -> { + hasAssets = true + } + } + } + + if (codeFiles.isNotEmpty() && annotationImpact.escalation(codeFiles.sorted()) != null) { + return BuildRoute.FullGradleBuild(InvalidationReason.ANNOTATION_PROCESSOR_INPUT_CHANGED) + } + + // Changed assets ride in EVERY route's deploy payload, so a device that cannot serve + // them makes any asset-bearing set stale - not just an assets-only one. + if (hasAssets && !assetsLiveReloadable) { + return BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED) + } + + return when { + codeFiles.isNotEmpty() && hasResources -> BuildRoute.CodeAndResources + codeFiles.isNotEmpty() -> BuildRoute.CodeOnly + hasResources -> BuildRoute.ResourcesOnly + hasAssets -> BuildRoute.AssetsOnly + else -> BuildRoute.NoOp + } + } + + private enum class FileKind { GRADLE_CONFIG, MANIFEST, CODE, RESOURCE, ASSET, UNSUPPORTED } + + companion object { + private val GRADLE_FILE_NAMES = + setOf( + "build.gradle", + "build.gradle.kts", + "settings.gradle", + "settings.gradle.kts", + "gradle.properties", + "local.properties", + ) + + private fun kindOf(file: File): FileKind { + val name = file.name + + if (name in GRADLE_FILE_NAMES || (name.endsWith(".toml") && hasSegment(file, "gradle"))) { + return FileKind.GRADLE_CONFIG + } + if (hasSegment(file, "wrapper") && name == "gradle-wrapper.properties") { + return FileKind.GRADLE_CONFIG + } + if (name == "AndroidManifest.xml") { + return FileKind.MANIFEST + } + + if (hasSourceSetDir(file, "res")) { + return FileKind.RESOURCE + } + if (hasSourceSetDir(file, "assets")) { + return FileKind.ASSET + } + if (name.endsWith(".kt") || name.endsWith(".java")) { + return FileKind.CODE + } + return FileKind.UNSUPPORTED + } + + /** + * True when [file] is [dir] or lives under it. + * + * @param file the changed path being classified; absolute from the watcher, + * relative in unit tests. + * @param dir the candidate ancestor, which must share a base with [file] because + * parent-chain entries are compared by equality, not canonicalized. + * @return true when [file] equals [dir] or [dir] appears in its parent chain. + */ + private fun isUnder( + file: File, + dir: File, + ): Boolean { + var current: File? = file + while (current != null) { + if (current == dir) return true + current = current.parentFile + } + return false + } + + /** + * True when [segment] appears as a whole path segment of [file]'s parent chain. + * + * @param file the changed path; only its directories are scanned, never its own name. + * @param segment one exact directory name to match, e.g. "gradle" or "wrapper". + * @return true when some ancestor directory of [file] is named [segment]. + */ + private fun hasSegment( + file: File, + segment: String, + ): Boolean { + var current: File? = file.parentFile + while (current != null) { + if (current.name == segment) return true + current = current.parentFile + } + return false + } + + /** + * True when [file] sits at `/src///...`. + * + * Anchored to that depth rather than scanning the whole parent chain because `res` and + * `assets` are legal package names: an unanchored scan reads + * `src/main/java/com/example/res/Strings.kt` as a resource, so aapt2 relinks, nothing + * compiles, and the user's edit is silently absent from the running app. + * + * @param file the changed path; only its directories are scanned, never its own name. + * @param segment the source-set child to match exactly, "res" or "assets". + * @return true when some ancestor is named [segment] and is a grandchild of a `src` dir. + */ + private fun hasSourceSetDir( + file: File, + segment: String, + ): Boolean { + var current: File? = file.parentFile + while (current != null) { + if (current.name == segment && current.parentFile?.parentFile?.name == "src") { + return true + } + current = current.parentFile + } + return false + } + + /** + * The source set [file] sits in - the `` of `/src//...`. + * + * Deliberately NOT folded into [kindOf], which also backs [hasRecognizedShape] and + * [namesResource]: a non-main file must keep its kind so a DELETION still routes rather + * than being dropped as noise, and so a `src/debug/res` diagnostic is still attributed to + * aapt2 instead of kotlinc. + * + * Shared with [TestSourceFilter], which drops test source sets upstream of this class. + * + * @param file the changed path; only its directories are scanned, never its own name. + * @return the innermost `src` child on its parent chain, or null when it has none, so a + * package named after a source set cannot rename the source set it is in. + */ + internal fun sourceSetName(file: File): String? { + var current: File? = file.parentFile + while (current != null) { + if (current.parentFile?.name == "src") return current.name + current = current.parentFile + } + return null + } + + /** + * True when [file]'s path shape alone names a role this classifier knows (Gradle config, + * manifest, code, resource, asset). No filesystem access - the file need not exist. + * + * Lets a caller drop a vanished path that never had a role, such as an atomic-rename + * tool's temp sibling (`sedXXXXXX`), as noise. + * + * @param file the path to weigh; usually one already deleted from disk. + * @return true when the shape names a known role, which alone does not force a Gradle + * fallback for a deletion - the caller still decides. + */ + fun hasRecognizedShape(file: File): Boolean = kindOf(file) != FileKind.UNSUPPORTED + + /** + * True when a vanished [file] names a project file rather than a rename tool's temp + * sibling, so its deletion must still route somewhere. + * + * Wider than [hasRecognizedShape] on purpose. An UNSUPPORTED path that carries an + * extension - a `.properties` under `src/main/resources`, a `.so` under `jniLibs` - is a + * real packaged file whose MODIFICATION already forces a Gradle fallback, so dropping its + * DELETION leaves the proxy app serving bytes the project no longer has. What the temps + * this filter exists for lack is any extension at all. + * + * @param file the vanished path to weigh; no filesystem access. + * @return true to keep the deletion, false to drop it as rename noise. + */ + fun namesProjectFile(file: File): Boolean = hasRecognizedShape(file) || file.extension.isNotEmpty() + + /** + * True when [file]'s path shape says it is an Android resource - under a source set's own + * `res/` directory, `src//res/...`. No filesystem access. + * + * Lets a caller attribute a diagnostic to aapt2 rather than kotlinc without plumbing the + * producing tool through the outcome: the two never mix, because a failed compile returns + * before the relink runs. + * + * @param file the path a diagnostic named. + * @return true when the path is a resource under the app's sources. + */ + fun namesResource(file: File): Boolean = kindOf(file) == FileKind.RESOURCE + } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md new file mode 100644 index 0000000000..df8d8d3653 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md @@ -0,0 +1,8 @@ +# `domain/classify/` - which build route a change takes + +Picks the cheapest still-correct build path for a coalesced changed-set, and names why a session baseline stops being trustworthy. Classification is by path shape, not file content (the one content-aware step is delegated to `domain/annotations/`). Pure logic, unit-testable without a project on disk. + +| File | Purpose | +| --- | --- | +| [`BuildRoute.kt`](BuildRoute.kt) | The route types (`FullGradleBuild`, `ResourcesOnly`, `AssetsOnly`, `CodeOnly`, `CodeAndResources`, `NoOp`, `WarmCompile`), the `recompilesCode` flag, and the `InvalidationReason` enum of why a baseline needs a full Gradle rebuild. | +| [`ChangeClassifier.kt`](ChangeClassifier.kt) | Routes a changed-set: manifest/Gradle-config/unsupported/non-app-module changes force a full build, otherwise splits code/resource/asset into the cheapest route; also exposes path-shape helpers (`hasRecognizedShape`, `namesResource`). | diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilter.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilter.kt new file mode 100644 index 0000000000..cc7fd3a552 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilter.kt @@ -0,0 +1,87 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import java.io.File + +/** + * Splits a watcher batch into the part a build should act on and the test-source saves it should + * ignore. + * + * Test source sets are watched but never built: the watch scope is the whole `src` tree (narrowing + * it would trade a wrong build for silent staleness elsewhere), while nothing under `src/test`, + * `src/androidTest` or `testFixtures` ships in the variant Quick Build deploys. Routing such a save + * to a full Gradle build was honest but useless - it spends minutes producing an app that cannot + * differ. Ignoring it outright is right, and the once-per-session notice is what keeps "ignored" + * from reading as "broken". + * + * Only test-type source sets. `src/debug` and flavor source sets DO ship in the built variant, so + * they keep their full-Gradle-build route. + */ +object TestSourceFilter { + /** + * The buildable remainder of [batch], and whether anything was dropped as a test source. + * + * @property buildable everything [batch] held that is not a test source; may be empty, which + * means the whole batch was test sources and no build should run at all. + * @property droppedTestSources whether at least one path was dropped, which is what the + * user is owed a notice about - true even when [buildable] still has work, since the + * dropped save did not deploy either way. + */ + data class Split( + val buildable: ChangedFiles.Known, + val droppedTestSources: Boolean, + ) + + /** + * Partitions one batch by source set. + * + * @param batch a reconciled watcher batch; modified and removed paths are split alike, since + * deleting a test file is no more deployable than saving one. + * @return the split; [Split.buildable] is never larger than [batch]. + */ + fun split(batch: ChangedFiles.Known): Split { + val modified = batch.files.filterNotTo(HashSet(), ::isTestSource) + val removed = batch.removed.filterNotTo(HashSet(), ::isTestSource) + val dropped = + modified.size != batch.files.size || removed.size != batch.removed.size + return Split(ChangedFiles.Known(modified, removed), dropped) + } + + /** + * True when [file] sits in a test-type source set, `/src//...`. + * + * Reads the source set the same way the classifier does - the innermost `src` child on the + * parent chain - so a package named `test` cannot make an ordinary source look like a test. + * + * @param file the changed path; no filesystem access, so it need not still exist. + * @return true when its source set is a test one, in any module. + */ + fun isTestSource(file: File): Boolean = ChangeClassifier.sourceSetName(file)?.let(::namesTestSourceSet) == true + + /** + * True when [name] is a source set AGP builds for tests rather than for the app. + * + * Matches on the camelCase boundary rather than a bare prefix, because AGP appends the flavor + * and build type (`testProDebug`, `androidTestDebug`) and a flavor may itself begin with the + * letters "test" - `testflavor` is a shipping source set and must not be ignored. + * + * @param name a source-set directory name, e.g. `main`, `debug`, `androidTestDebug`. + * @return true for `test*`, `androidTest*` and `testFixtures*`; false for `main`, `debug` and + * every flavor source set. + */ + private fun namesTestSourceSet(name: String): Boolean = hasCamelPrefix(name, "test") || hasCamelPrefix(name, "androidTest") + + /** + * True when [name] is exactly [prefix] or continues it at a camelCase boundary. + * + * @param name the source-set name to test. + * @param prefix the lower-camel prefix that identifies the family. + */ + private fun hasCamelPrefix( + name: String, + prefix: String, + ): Boolean { + if (!name.startsWith(prefix)) return false + return name.length == prefix.length || name[prefix.length].isUpperCase() + } +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescing.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescing.kt new file mode 100644 index 0000000000..4232bdebdf --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescing.kt @@ -0,0 +1,143 @@ +package org.appdevforall.cotg.quickbuild.domain.watch + +import kotlinx.coroutines.Job +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.channelFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import java.io.File + +/** + * One raw watcher observation before coalescing: a path was written or created ([Modified]), + * or deleted ([Removed]). + * + * The two are distinguished at the source because a standalone deletion (`git pull`, + * branch-switch, `rm`) fires no create-or-modify event, so it would otherwise never reach the + * build pipeline. + */ +sealed interface WatchEvent { + /** The path the observation is about, as the watcher reported it (absolute on device). */ + val file: File + + /** + * The path was written or created, so its current bytes are on disk for the build to read. + * + * @property file the written path; a create and a rewrite are not distinguished, since both + * feed the compiler the same way. + */ + data class Modified( + override val file: File, + ) : WatchEvent + + /** + * The path was deleted. + * + * @property file the deleted path; nothing is left on disk, so it can only be classified by + * the shape of the path itself. + */ + data class Removed( + override val file: File, + ) : WatchEvent +} + +/** + * Coalesces a stream of file-change events into batches, so a burst of writes (save-all, git pull, + * codegen) becomes one quick build instead of many. + * + * Each batch carries modified and removed paths together, and the last event per path wins: + * create-then-delete collapses to a removal, delete-then-recreate to a modification. + * + * @param quietMillis emit this long after the LAST event; every new event resets the timer. + * @param maxMillis hard cap measured from the FIRST event of the batch, so a long continuous write + * stream still fires promptly and stragglers land in the follow-up build. + * @return one [ChangedFiles.Known] per quiet-period or cap expiry, never an empty batch; the + * upstream's completion flushes whatever is still accumulating. + */ +fun Flow.coalesceChanges( + quietMillis: Long, + maxMillis: Long, +): Flow = + channelFlow { + // Keyed by path so the last event for a path wins (create-then-delete -> removed). + val batch = LinkedHashMap() + val lock = Mutex() + var quietTimer: Job? = null + var capTimer: Job? = null + + suspend fun flush() { + // flush() usually runs inside one of the timer jobs, and must never cancel the job + // executing it: the send() below would then throw CancellationException as soon as + // it had to suspend on a busy consumer, silently dropping the batch. + val self = currentCoroutineContext()[Job] + val snapshot = + lock.withLock { + if (quietTimer !== self) quietTimer?.cancel() + quietTimer = null + if (capTimer !== self) capTimer?.cancel() + capTimer = null + if (batch.isEmpty()) null else LinkedHashMap(batch).also { batch.clear() } + } + // Send outside the lock so a slow consumer never stalls the collector's timers. + if (snapshot != null) { + send(snapshot.toChangedFiles()) + } + } + + collect { event -> + val startedBatch = + lock.withLock { + val first = batch.isEmpty() + batch[event.file] = event + quietTimer?.cancel() + quietTimer = + launch { + delay(quietMillis) + flush() + } + first + } + if (startedBatch) { + // Cap timer is armed once per batch on the first event and never reset. + lock.withLock { + capTimer?.cancel() + capTimer = + launch { + delay(maxMillis) + flush() + } + } + } + } + + // Upstream completed: emit whatever is still pending so nothing is dropped. + flush() + } + +private fun Map.toChangedFiles(): ChangedFiles.Known { + val modified = LinkedHashSet() + val removed = LinkedHashSet() + for ((file, event) in this) { + when (event) { + is WatchEvent.Modified -> modified.add(file) + is WatchEvent.Removed -> removed.add(file) + } + } + return ChangedFiles.Known(modified, removed) +} + +/** Default debounce for the on-device project watcher. */ +object ChangeCoalescingDefaults { + /** Quiet period after the last event; short enough that a save still feels immediate. */ + const val QUIET_MILLIS = 150L + + /** Cap from the batch's first event, so a continuous write stream cannot defer a build. */ + const val MAX_MILLIS = 1_000L + + /** Channel capacity for the raw pre-coalesce event stream; a burst buffers, never blocks. */ + const val RAW_EVENT_BUFFER = Channel.UNLIMITED +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/README.md b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/README.md new file mode 100644 index 0000000000..b35c00e521 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/README.md @@ -0,0 +1,9 @@ +# `domain/watch/` - what counts as a change + +Turns the raw watcher event stream into clean, deduplicated build batches. Decides which filesystem events are relevant, debounces a burst of writes into one batch, and reconciles paths that vanished between the event and the build. Pure JVM (a coroutine clock, unit-tested with virtual time); no Android. + +| File | Purpose | +| --- | --- | +| [`WatchFilter.kt`](WatchFilter.kt) | Decides if an event is relevant: under a watched `src/`/`res/`/`assets/` root or a watched Gradle file, not a `build/` intermediate, not a recognized-shape temp file. | +| [`ChangeCoalescing.kt`](ChangeCoalescing.kt) | Defines `WatchEvent` (Modified/Removed) and `coalesceChanges`, which debounces events into batches (quiet timer plus a hard cap from the first event), last-event-per-path wins. | +| [`WatcherBatchReconciler.kt`](WatcherBatchReconciler.kt) | Splits a coalesced batch into files that still exist, deletions, and noise; a modified-but-gone path with a recognized shape becomes a removal, otherwise it is dropped as a rename-tool temp. | diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilter.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilter.kt new file mode 100644 index 0000000000..4281e089d2 --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilter.kt @@ -0,0 +1,102 @@ +package org.appdevforall.cotg.quickbuild.domain.watch + +import java.io.File + +/** + * Decides whether a filesystem event is relevant to the quick-build session: inside the watched + * `src/`, `res/` and `assets/` roots or the watched Gradle files, and not a build intermediate or + * a temp file. The temp names dropped here come from EXTERNAL atomic-rename tools (`sed -i`, `git + * checkout`/`stash`, vim with `backupcopy=yes`); only recognized shapes are dropped here, an + * unrecognized one such as `sed`'s `sedXXXXXX` later, by [WatcherBatchReconciler]. + * + * @param watchedRoots directories whose subtrees are relevant, `build/` excepted; resolved to + * absolute paths once at construction, so later relative-path callers still match. + * @param watchedFiles individual files that are relevant wherever they sit (the manifest and the + * gradle files), matched exactly rather than by subtree. + */ +class WatchFilter( + watchedRoots: Collection, + watchedFiles: Collection = emptyList(), +) { + private val roots = watchedRoots.map { it.absoluteFile } + private val files = watchedFiles.mapTo(HashSet()) { it.absoluteFile } + + /** + * True when the session should react to a change at [file]. + * + * @param file the changed path, absolute or relative; it need not still exist, since deletions + * are filtered by the same rules. + * @return true to pass the event to the session; false drops it silently, so a watched file + * wrongly excluded here becomes a stale build with no warning. + */ + fun isRelevant(file: File): Boolean { + val abs = file.absoluteFile + if (isTempArtifact(abs.name)) return false + if (abs in files) return true + + val underRoot = roots.any { root -> abs.startsWith(root) } + if (!underRoot) return false + return !hasBuildSegment(abs) + } + + /** + * True when [root] is this file or one of its ancestor directories. + * + * @receiver an absolute path, so the walk terminates at the filesystem root. + * @param root an already-absolute watched root; equality with the receiver counts as a match. + * @return true when the receiver lies in [root]'s subtree, comparing path segments only - no + * symlink resolution, so a link into a watched root does not match. + */ + private fun File.startsWith(root: File): Boolean { + var current: File? = this + while (current != null) { + if (current == root) return true + current = current.parentFile + } + return false + } + + /** + * True when the path passes through a `build/` dir OUTSIDE any `src/` (Gradle intermediates). + * + * The walk stops at the `src` boundary because Gradle's `build/` is a module-root sibling of + * `src/`, never inside it, while `build` is a legal Kotlin/Java package name: an unbounded walk + * drops `src/main/java/com/example/build/Builders.kt` upstream of both the inotify and poll + * channels, so that save reaches nothing at all - no build, no batch, no warning. + * + * @param file the changed path; only its ancestors are examined, so a source file itself named + * `build` is not excluded. + * @return true to exclude the path as a build intermediate. + */ + private fun hasBuildSegment(file: File): Boolean { + var current: File? = file.parentFile + var sawBuild = false + while (current != null) { + // Reached from below, so a `src` ancestor proves every `build` seen so far sits + // inside a source set and is therefore a package, not an intermediate. + if (current.name == "src") return false + if (current.name == "build") sawBuild = true + current = current.parentFile + } + return sawBuild + } + + /** + * True for names an editor or rename-based tool leaves behind rather than real sources. + * + * @param name the file's simple name, never a path - every test here is a prefix or suffix + * match on that name alone. + * @return true to drop the event; unrecognized temp shapes return false and are dropped later, + * at batch-settle time. + */ + private fun isTempArtifact(name: String): Boolean = + name.startsWith(".") || + name.endsWith("~") || + name.endsWith(".tmp") || + name.endsWith(".swp") || + name.endsWith(".bak") || + // A persisted `patch`/merge dropping under `src/` would otherwise classify + // UNSUPPORTED and force a spurious rebaseline. + name.endsWith(".orig") || + name.endsWith(".rej") +} diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconciler.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconciler.kt new file mode 100644 index 0000000000..188612a06e --- /dev/null +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconciler.kt @@ -0,0 +1,46 @@ +package org.appdevforall.cotg.quickbuild.domain.watch + +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.classify.ChangeClassifier +import java.io.File + +/** + * Reconciles a raw watcher batch into the modified/removed split the pipeline builds against. + * + * A path reported as modified but already gone is reclassified: if it names a project file it is a + * deletion the modify channel caught (a `git checkout` rename whose target was then dropped), and + * otherwise it is a rename-tool temp dropped as noise - without that, a stray temp would push the + * whole batch to a spurious + * [org.appdevforall.cotg.quickbuild.domain.classify.BuildRoute.FullGradleBuild]. + * + * "Names a project file" is [ChangeClassifier.namesProjectFile], not the narrower recognized-shape + * test: an extension-bearing path the classifier calls UNSUPPORTED is still a packaged file, and + * dropping its deletion leaves the proxy app serving content the project no longer has. + */ +object WatcherBatchReconciler { + /** + * Splits [batch] into the files that still exist, the deletions, and the noise to drop. + * + * @param batch the coalesced watcher batch, whose `files` may name paths already gone. + * @param exists whether the path is currently a live file; production passes + * `File.isFile` (a path that turned into a directory counts as vanished). + * @return the same batch with vanished paths moved to `removed` or dropped; never larger than + * [batch]. + */ + fun reconcile( + batch: ChangedFiles.Known, + exists: (File) -> Boolean, + ): ChangedFiles.Known { + val modified = HashSet() + val removed = HashSet() + batch.removed.filterTo(removed, ChangeClassifier::namesProjectFile) + for (file in batch.files) { + when { + exists(file) -> modified.add(file) + ChangeClassifier.namesProjectFile(file) -> removed.add(file) + // else: unrecognized vanished temp -> drop as noise. + } + } + return ChangedFiles.Known(modified, removed) + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/OfflineNetworkGuardTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/OfflineNetworkGuardTest.kt new file mode 100644 index 0000000000..fb0467765d --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/OfflineNetworkGuardTest.kt @@ -0,0 +1,55 @@ +package org.appdevforall.cotg.quickbuild + +import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.assertWithMessage +import org.appdevforall.cotg.quickbuild.testfixtures.OfflineGuard +import org.junit.jupiter.api.Test + +/** + * Offline guard (ADFA-4128 offline-test-plan touchpoints 7-10) for the IDE-side + * session/orchestration/deploy code: scans this module's compiled production classes for + * network-API references in their constant pools, naming any offender. + * + * No allowed exceptions here, unlike :quickbuild:daemon. `java/net/URL`/`URI`/ + * `URLClassLoader` are not in [OfflineGuard.BANNED], so a local `file:` URI would pass. + */ +class OfflineNetworkGuardTest { + @Test + fun productionClassesReferenceNoNetworkApis() { + val buildDir = OfflineGuard.moduleBuildDir(javaClass) + val classFiles = OfflineGuard.productionClassFiles(buildDir) + + // Anti-vacuous: a mis-location must fail loudly, never pass by scanning nothing. + assertWithMessage("no production .class files found under $buildDir -- guard self-location is broken") + .that(classFiles) + .isNotEmpty() + + val violations = OfflineGuard.scanForBannedReferences(buildDir, classFiles) + assertWithMessage( + "Quick Build must be network-free offline, but production classes reference banned network APIs:\n" + + violations.joinToString("\n") { " - $it" } + + "\n(scanned ${classFiles.size} classes under $buildDir)", + ).that(violations) + .isEmpty() + } + + /** + * Proves the detector would genuinely fail if a banned reference appeared, and that + * the allow-listed local-URL APIs do NOT trip it -- so a green result above is a real + * signal, not a scanner that can never fire. + */ + @Test + fun detectorFiresOnBannedBytesAndNotOnAllowedBytes() { + val banned = + "prefix Lokhttp3/OkHttpClient; and java/net/Socket suffix" + .toByteArray(Charsets.US_ASCII) + assertThat(OfflineGuard.BANNED.filter { OfflineGuard.containsAscii(banned, it) }) + .containsExactly("okhttp3/", "java/net/Socket") + + val allowed = + "Ljava/net/URL; Ljava/net/URLClassLoader; Ljava/net/URI;" + .toByteArray(Charsets.US_ASCII) + assertThat(OfflineGuard.BANNED.filter { OfflineGuard.containsAscii(allowed, it) }) + .isEmpty() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt new file mode 100644 index 0000000000..486f931159 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt @@ -0,0 +1,138 @@ +package org.appdevforall.cotg.quickbuild.data + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.watch.WatchFilter +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File + +/** + * Lifecycle and choke-point edges of [AndroidProjectWatcher] beyond + * [AndroidProjectWatcherTest]'s pipeline cases, driven through the same JVM seams + * ([AndroidProjectWatcher.report] / [AndroidProjectWatcher.sweep]) on the same virtual clock. + */ +class AndroidProjectWatcherEdgeTest { + @TempDir lateinit var tempDir: File + + private fun TestScope.startWatcher( + root: File, + batches: MutableList, + pollIntervalMillis: Long = 3_600_000L, // parked; sweeps are driven manually + ): AndroidProjectWatcher { + val watcher = + AndroidProjectWatcher( + watchedRoots = listOf(root), + watchedFiles = emptyList(), + filter = WatchFilter(listOf(root)), + // backgroundScope so the never-ending poll job is cancelled with the test. + scope = backgroundScope, + pollIntervalMillis = pollIntervalMillis, + quietMillis = QUIET_MILLIS, + maxMillis = MAX_MILLIS, + pollDispatcher = StandardTestDispatcher(testScheduler), + ) + watcher.start(batches::add) + // Prime the poll's baseline before the test touches anything. + runCurrent() + return watcher + } + + /** Advances past the quiet window and the cap, so every pending batch has been emitted. */ + private fun TestScope.settle() { + advanceTimeBy(MAX_MILLIS + 1) + runCurrent() + } + + @Test + fun `stop before start is a safe no-op`() = + runTest { + val watcher = + AndroidProjectWatcher( + watchedRoots = listOf(tempDir), + watchedFiles = emptyList(), + filter = WatchFilter(listOf(tempDir)), + scope = backgroundScope, + ) + + // Nothing was started; stop must not throw on the never-armed jobs. + watcher.stop() + } + + @Test + fun `a directory event is dropped at the choke point - never a compile input`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val srcDir = File(root, "app/src/main/java/com/example").apply { mkdirs() } + val source = File(srcDir, "Foo.kt").apply { writeText("class Foo") } + val batches = mutableListOf() + val watcher = startWatcher(root, batches) + + // A directory "change" (as inotify would deliver for a mkdir) must not emit... + watcher.report(srcDir, fromPoll = false) + // ...while a real file change right after emits normally. + watcher.report(source, fromPoll = false) + settle() + + assertThat(batches.single().files).containsExactly(source) + } + + @Test + fun `the automatic poll loop sweeps a change to a batch without a manual sweep`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val source = + File(root, "app/src/main/java/com/example/Foo.kt").apply { + parentFile!!.mkdirs() + writeText("class Foo") + } + val batches = mutableListOf() + // A real, running loop - this is the case under test, so its interval is live. + val pollIntervalMillis = 50L + startWatcher(root, batches, pollIntervalMillis) + + // A content change the (inert) inotify path never reports: only the poll's own + // recurring sweep can deliver it. + source.writeText("class Foo { val added = 1 }") + advanceTimeBy(pollIntervalMillis + 1) + settle() + + assertThat(batches.single().files).containsExactly(source) + } + + @Test + fun `a poll observation of an unchanged file stays quiet`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val batches = mutableListOf() + val watcher = startWatcher(root, batches) + // Created after the baseline priming, so it is a genuinely NEW path to the poll. + val source = + File(root, "app/src/main/java/com/example/Foo.kt").apply { + parentFile!!.mkdirs() + writeText("class Foo") + } + + // First poll sighting of the new path records the fingerprint and emits... + watcher.report(source, fromPoll = true) + settle() + assertThat(batches).hasSize(1) + + // ...but a second sweep over the untouched file must NOT re-emit (the + // fingerprint gate is what keeps the hybrid from double-building). + watcher.report(source, fromPoll = true) + settle() + + assertThat(batches).hasSize(1) + } + + private companion object { + private const val QUIET_MILLIS = 50L + private const val MAX_MILLIS = 1_000L + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherTest.kt new file mode 100644 index 0000000000..b80f902d46 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherTest.kt @@ -0,0 +1,210 @@ +package org.appdevforall.cotg.quickbuild.data + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.watch.WatchFilter +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File + +/** + * JVM tests for the watcher's poll/coalesce pipeline. FileObserver is inert on the JVM, so + * inotify deliveries are simulated via [AndroidProjectWatcher.report] and sweeps driven via + * [AndroidProjectWatcher.sweep], on the virtual clock: a "stayed quiet" assertion means the + * pipeline had nothing left to do. Regression pinned: `adb push` back-dates mtime after + * CLOSE_WRITE, so the next sweep emits a phantom second batch - a duplicate rebaseline. + */ +class AndroidProjectWatcherTest { + @TempDir lateinit var tempDir: File + + private fun TestScope.startWatcher( + root: File, + batches: MutableList, + ): AndroidProjectWatcher { + val watcher = + AndroidProjectWatcher( + watchedRoots = listOf(root), + watchedFiles = emptyList(), + filter = WatchFilter(listOf(root)), + // backgroundScope so the never-ending poll job is cancelled with the test. + scope = backgroundScope, + // Park the automatic sweep; tests call sweep() deterministically. + pollIntervalMillis = 3_600_000L, + quietMillis = QUIET_MILLIS, + maxMillis = MAX_MILLIS, + pollDispatcher = StandardTestDispatcher(testScheduler), + ) + watcher.start { batches += it } + // Run the poll loop's initFingerprints() pass before any edit, so the fingerprint + // state matches a long-running session's. + runCurrent() + return watcher + } + + /** Advances past the quiet window and the cap, so every pending batch has been emitted. */ + private fun TestScope.settle() { + advanceTimeBy(MAX_MILLIS + 1) + runCurrent() + } + + @Test + fun `post-write mtime settle does not re-emit the same edit via the poll`() = + runTest { + val root = File(tempDir, "src").apply { mkdirs() } + val manifest = + File(root, "main/AndroidManifest.xml").apply { + parentFile!!.mkdirs() + writeText("") + } + val batches = mutableListOf() + val watcher = startWatcher(root, batches) + + // The adb-push shape: write, inotify CLOSE_WRITE fingerprints current attrs, + // then utimensat back-dates mtime with no further masked event. + manifest.writeText("") + watcher.report(manifest, fromPoll = false) + assertThat(manifest.setLastModified(manifest.lastModified() - 7_000)).isTrue() + + settle() + assertThat(batches.single().files).containsExactly(manifest) + + // The poll sweep after the batch settled must stay quiet: the edit was already + // delivered, only its attrs moved. A second batch here is a phantom, and costs a + // double invalidation/rebaseline. + watcher.sweep() + settle() + assertThat(batches).hasSize(1) + } + + @Test + fun `poll still catches a real change whose inotify events were dropped`() = + runTest { + val root = File(tempDir, "src").apply { mkdirs() } + val source = + File(root, "main/java/A.kt").apply { + parentFile!!.mkdirs() + writeText("class A") + } + val batches = mutableListOf() + val watcher = startWatcher(root, batches) + + // A delivered edit settles as batch 1. + source.writeText("class A { fun a() = 1 }") + watcher.report(source, fromPoll = false) + settle() + assertThat(batches).hasSize(1) + + // A later REAL write with every inotify event dropped (sdcardfs): only the + // poll can see it. The settle-time re-stamp must not have eaten this. + source.writeText("class A { fun a() = 1; fun b() = 2 }") + watcher.sweep() + settle() + assertThat(batches).hasSize(2) + assertThat(batches[1].files).containsExactly(source) + + // And once delivered, a further sweep with no change stays quiet. + watcher.sweep() + settle() + assertThat(batches).hasSize(2) + } + + @Test + fun `file deleted before the batch settles still reaches the pipeline once`() = + runTest { + val root = File(tempDir, "src").apply { mkdirs() } + val source = + File(root, "main/java/B.kt").apply { + parentFile!!.mkdirs() + writeText("class B") + } + val batches = mutableListOf() + val watcher = startWatcher(root, batches) + + source.writeText("class B { }") + watcher.report(source, fromPoll = false) + // Gone before the quiet window elapses: the settle re-stamp must skip it, and + // the poll's set-diff then emits the removal exactly once. + assertThat(source.delete()).isTrue() + settle() + assertThat(batches.single().files).containsExactly(source) + + watcher.sweep() + settle() + assertThat(batches).hasSize(2) + assertThat(batches[1].removed).containsExactly(source) + + watcher.sweep() + settle() + assertThat(batches).hasSize(2) + } + + @Test + fun `a live file inotify fingerprinted mid-walk is not reported as removed`() = + runTest { + // The sweep's set-diff races the inotify path: fingerprints is written by both, so a + // file inotify records after the walk passed its directory is in the map and absent + // from the walk while alive on disk. Reproduced deterministically by fingerprinting a + // file the walk cannot reach - same state, no threads. + val walked = File(tempDir, "walked").apply { mkdirs() } + val unwalked = + File(tempDir, "unwalked/main/java/Live.kt").apply { + parentFile!!.mkdirs() + writeText("class Live") + } + val batches = mutableListOf() + val watcher = + AndroidProjectWatcher( + watchedRoots = listOf(walked), + watchedFiles = emptyList(), + // Wider than the walked root, so report() accepts a path the sweep never sees. + filter = WatchFilter(listOf(tempDir)), + scope = backgroundScope, + pollIntervalMillis = 3_600_000L, + quietMillis = QUIET_MILLIS, + maxMillis = MAX_MILLIS, + pollDispatcher = StandardTestDispatcher(testScheduler), + ) + watcher.start { batches += it } + runCurrent() + + watcher.report(unwalked, fromPoll = false) + settle() + assertThat(batches.single().files).containsExactly(unwalked) + + watcher.sweep() + settle() + + // A phantom removal here is not merely noise: coalescing is last-event-wins, so it + // would hand the daemon a REMOVED file that is still in allSources, and the dropped + // fingerprint makes the next sweep re-emit the same edit - one save, two builds. + assertThat(batches.flatMap { it.removed }).doesNotContain(unwalked) + assertThat(unwalked.isFile).isTrue() + } + + @Test + fun `a created directory tree registers one watch per directory, recursively`() = + runTest { + // A directory created mid-session used to get a watch whose own CREATE handler did + // not recurse, so anything created two levels down fell back to the 2s poll. + val root = File(tempDir, "src").apply { mkdirs() } + val batches = mutableListOf() + val watcher = startWatcher(root, batches) + val before = watcher.watchCount() + + File(root, "main/java/com/example").apply { mkdirs() } + watcher.registerCreatedTree(File(root, "main")) + + // main, main/java, main/java/com, main/java/com/example. + assertThat(watcher.watchCount() - before).isEqualTo(4) + } + + private companion object { + private const val QUIET_MILLIS = 60L + private const val MAX_MILLIS = 500L + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/ChangedFilesTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/ChangedFilesTest.kt new file mode 100644 index 0000000000..b973b78322 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/ChangedFilesTest.kt @@ -0,0 +1,80 @@ +package org.appdevforall.cotg.quickbuild.domain + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test +import java.io.File + +class ChangedFilesTest { + private fun known(vararg paths: String) = ChangedFiles.Known(paths.map(::File).toSet()) + + private fun removed(vararg paths: String) = ChangedFiles.Known(emptySet(), paths.map(::File).toSet()) + + @Test + fun `union of known sets is the set union`() { + val union = known("a.kt", "b.kt") + known("b.kt", "c.kt") + + assertThat(union).isEqualTo(known("a.kt", "b.kt", "c.kt")) + } + + @Test + fun `union unions the removed sets independently of the modified sets`() { + val union = (known("a.kt") + removed("old.kt")) + (known("b.kt") + removed("gone.kt")) + + assertThat(union).isEqualTo(ChangedFiles.Known(setOf(File("a.kt"), File("b.kt")), setOf(File("old.kt"), File("gone.kt")))) + } + + @Test + fun `a path modified in one batch then deleted in the newer batch collapses to a removal`() { + // Right operand is the newer batch at every union site. A plain set union would leave + // x.kt in BOTH sets and the executor would feed it to the daemon as changed AND removed. + val union = known("x.kt", "a.kt") + removed("x.kt") + + assertThat(union).isEqualTo(ChangedFiles.Known(setOf(File("a.kt")), setOf(File("x.kt")))) + } + + @Test + fun `a path deleted in one batch then recreated in the newer batch collapses to a modification`() { + val union = removed("x.kt", "gone.kt") + known("x.kt") + + assertThat(union).isEqualTo(ChangedFiles.Known(setOf(File("x.kt")), setOf(File("gone.kt")))) + } + + @Test + fun `union of batches with disjoint sets never lands a path in both sets`() { + val union = ((known("a.kt") + removed("old.kt")) + (known("b.kt") + removed("gone.kt"))) as ChangedFiles.Known + + assertThat(union.files.intersect(union.removed)).isEmpty() + } + + @Test + fun `a set with only removals is not empty`() { + assertThat(removed("gone.kt").isEmpty).isFalse() + } + + @Test + fun `unknown absorbs a removals-only known`() { + assertThat(removed("gone.kt") + ChangedFiles.Unknown).isEqualTo(ChangedFiles.Unknown) + } + + @Test + fun `unknown absorbs known on either side`() { + assertThat(known("a.kt") + ChangedFiles.Unknown).isEqualTo(ChangedFiles.Unknown) + assertThat(ChangedFiles.Unknown + known("a.kt")).isEqualTo(ChangedFiles.Unknown) + assertThat(ChangedFiles.Unknown + ChangedFiles.Unknown).isEqualTo(ChangedFiles.Unknown) + } + + @Test + fun `empty known set is empty but unknown is not`() { + assertThat(ChangedFiles.Known.EMPTY.isEmpty).isTrue() + assertThat(known("a.kt").isEmpty).isFalse() + assertThat(ChangedFiles.Unknown.isEmpty).isFalse() + } + + @Test + fun `union with empty is identity`() { + val set = known("a.kt") + + assertThat(set + ChangedFiles.Known.EMPTY).isEqualTo(set) + assertThat(ChangedFiles.Known.EMPTY + set).isEqualTo(set) + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt new file mode 100644 index 0000000000..5755130c68 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt @@ -0,0 +1,507 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import com.google.common.truth.Truth.assertThat +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.classify.BuildRoute +import org.appdevforall.cotg.quickbuild.domain.classify.ChangeClassifier +import org.appdevforall.cotg.quickbuild.domain.classify.InvalidationReason +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File + +/** + * The correctness contract of annotation-aware classification, exercised against a + * realistic Room + Hilt fixture on disk. + * + * The asymmetry to keep in mind while reading: a wrong "safe" ships stale generated code + * (a never-stale violation), while a wrong "rebaseline" only costs ~8 s. Every ambiguous + * case below therefore asserts the rebaseline. + */ +class AnnotationImpactAnalyzerTest { + @TempDir + lateinit var root: File + + private val roomProfile = AnnotationProcessorProfile.of(listOf("androidx.room:room-compiler:2.6.1")) + + private fun analyzer( + fixture: RoomAppFixture, + profile: AnnotationProcessorProfile = roomProfile, + ): AnnotationImpactAnalyzer = AnnotationImpactAnalyzer(profile, AnnotationBaseline.capture(fixture.all, profile)) + + private fun fixture(): RoomAppFixture = RoomAppFixture(root) + + @Test + fun `no processors configured is inactive and never escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture, AnnotationProcessorProfile.NONE) + fixture.edit(fixture.userDao, RoomAppFixture.USER_DAO.replace("ORDER BY name", "ORDER BY id")) + + assertThat(analyzer.active).isFalse() + assertThat(analyzer.escalation(listOf(fixture.userDao))).isNull() + } + + @Test + fun `unedited annotated file does not escalate`() { + val fixture = fixture() + assertThat(analyzer(fixture).escalation(fixture.all)).isNull() + } + + @Test + fun `editing Query SQL escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit(fixture.userDao, RoomAppFixture.USER_DAO.replace("ORDER BY name", "ORDER BY id")) + + assertThat(analyzer.escalation(listOf(fixture.userDao))).contains("UserDao.kt") + } + + @Test + fun `adding an entity column escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.user, + RoomAppFixture.USER.replace("\tval name: String,", "\tval name: String,\n\tval nickname: String,"), + ) + + assertThat(analyzer.escalation(listOf(fixture.user))).isNotNull() + } + + @Test + fun `adding a Dao method escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.userDao, + RoomAppFixture.USER_DAO.replace( + "\t@Insert", + "\t@Query(\"SELECT COUNT(*) FROM users\")\n\tfun count(): Int\n\n\t@Insert", + ), + ) + + assertThat(analyzer.escalation(listOf(fixture.userDao))).isNotNull() + } + + @Test + fun `removing an annotation escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit(fixture.user, RoomAppFixture.USER.replace("@PrimaryKey val id", "val id")) + + assertThat(analyzer.escalation(listOf(fixture.user))).isNotNull() + } + + @Test + fun `new file carrying an entity annotation escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + val note = + fixture.write( + "Note.kt", + """ + package com.example.notes + + import androidx.room.Entity + import androidx.room.PrimaryKey + + @Entity + data class Note(@PrimaryKey val id: Long, val body: String) + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(note))).contains("new file") + } + + @Test + fun `new file without processor annotations stays on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + val helper = + fixture.write( + "Strings.kt", + """ + package com.example.notes + + object Strings { + fun shout(value: String): String { + return value.uppercase() + } + } + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(helper))).isNull() + } + + @Test + fun `deleting an annotated file escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + assertThat(fixture.userDao.delete()).isTrue() + + assertThat(analyzer.escalation(listOf(fixture.userDao))).contains("deleted") + } + + @Test + fun `an anchor file reported as changed but byte-identical stays on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + // A watcher event with no real content change (touch, editor re-save). + fixture.edit(fixture.address, RoomAppFixture.ADDRESS) + + assertThat(analyzer.escalation(listOf(fixture.address))).isNull() + } + + @Test + fun `deleting an anchor file escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + assertThat(fixture.address.delete()).isTrue() + + assertThat(analyzer.escalation(listOf(fixture.address))).contains("Address") + } + + @Test + fun `adding a declaration to an anchor file escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.baseEntity, + RoomAppFixture.BASE_ENTITY.replace( + "var createdAt: Long = 0", + "var createdAt: Long = 0\n\n\tfun touch() {\n\t\tcreatedAt = 1\n\t}", + ), + ) + + assertThat(analyzer.escalation(listOf(fixture.baseEntity))).isNotNull() + } + + @Test + fun `deleting a plain file stays on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + assertThat(fixture.formatter.delete()).isTrue() + + assertThat(analyzer.escalation(listOf(fixture.formatter))).isNull() + } + + @Test + fun `body-only edit of a plain UI file stays on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit(fixture.activity, RoomAppFixture.ACTIVITY.replace("\"Notes\"", "\"My Notes\"")) + + assertThat(analyzer.escalation(listOf(fixture.activity))).isNull() + } + + @Test + fun `body-only edit inside an annotated file stays on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.converters, + RoomAppFixture.CONVERTERS.replace("return value?.toString()", "return value?.toString()?.trim()"), + ) + + assertThat(analyzer.escalation(listOf(fixture.converters))).isNull() + } + + @Test + fun `changing a converter signature escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.converters, + RoomAppFixture.CONVERTERS.replace("fun fromTimestamp(value: Long?): String?", "fun fromTimestamp(value: Int?): String?"), + ) + + assertThat(analyzer.escalation(listOf(fixture.converters))).isNotNull() + } + + @Test + fun `comment and whitespace edits stay on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.userDao, + RoomAppFixture.USER_DAO + .replace("@Dao", "/** The users table. */\n@Dao") + .replace("interface UserDao {", "interface UserDao {\n"), + ) + + assertThat(analyzer.escalation(listOf(fixture.userDao))).isNull() + } + + @Test + fun `import-only change that brings nothing processor-relevant into scope stays on the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.viewModel, + RoomAppFixture.VIEW_MODEL.replace("package com.example.notes", "package com.example.notes\n\nimport kotlin.math.max"), + ) + + assertThat(analyzer.escalation(listOf(fixture.viewModel))).isNull() + } + + @Test + fun `import change that brings a Room annotation into scope escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.formatter, + """ + package com.example.notes + + import androidx.room.Entity + + @Entity + data class Formatted(val id: Long) + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(fixture.formatter))).isNotNull() + } + + @Test + fun `editing an Embedded value type escalates even though it has no annotation`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit(fixture.address, RoomAppFixture.ADDRESS.replace("val city: String,", "val city: String,\n\tval zip: String,")) + + assertThat(analyzer.escalation(listOf(fixture.address))).contains("Address") + } + + @Test + fun `editing a non-annotated entity base class escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit( + fixture.baseEntity, + RoomAppFixture.BASE_ENTITY.replace("var createdAt: Long = 0", "var createdAt: Long = 0\n\tvar updatedAt: Long = 0"), + ) + + assertThat(analyzer.escalation(listOf(fixture.baseEntity))).contains("BaseEntity") + } + + @Test + fun `a batch escalates when any one file touches processor input`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit(fixture.activity, RoomAppFixture.ACTIVITY.replace("\"Notes\"", "\"My Notes\"")) + fixture.edit(fixture.userDao, RoomAppFixture.USER_DAO.replace("ORDER BY name", "ORDER BY id")) + + assertThat(analyzer.escalation(listOf(fixture.activity, fixture.userDao))).isNotNull() + } + + @Test + fun `an unscannable edit escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + // Mid-typing state: the closing brace has not been typed yet. + fixture.edit(fixture.activity, RoomAppFixture.ACTIVITY.dropLast(1)) + + assertThat(analyzer.escalation(listOf(fixture.activity))).contains("could not be scanned") + } + + @Test + fun `an unrecognized processor treats any annotation as input`() { + val fixture = fixture() + val profile = AnnotationProcessorProfile.of(listOf("com.example:mystery-processor:1.0")) + val analyzer = analyzer(fixture, profile) + fixture.edit( + fixture.formatter, + """ + package com.example.notes + + import com.example.mystery.Magic + + @Magic + object Formatter { + fun format(name: String): String { + return name.trim() + } + } + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(fixture.formatter))).isNotNull() + } + + @Test + fun `an unrecognized processor still live-reloads a file with no annotations`() { + val fixture = fixture() + val profile = AnnotationProcessorProfile.of(listOf("com.example:mystery-processor:1.0")) + val analyzer = analyzer(fixture, profile) + fixture.edit(fixture.formatter, RoomAppFixture.FORMATTER.replace("name.trim()", "name.trim().lowercase()")) + + assertThat(analyzer.escalation(listOf(fixture.formatter))).isNull() + } + + @Test + fun `an unrecognized processor ignores language-level annotations`() { + val fixture = fixture() + val profile = AnnotationProcessorProfile.of(listOf("com.example:mystery-processor:1.0")) + val analyzer = analyzer(fixture, profile) + fixture.edit( + fixture.formatter, + """ + package com.example.notes + + object Formatter { + @Deprecated("use format2") + fun format(name: String): String { + return name.trim() + } + } + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(fixture.formatter))).isNull() + } + + @Test + fun `reverting an annotation edit returns to the live reload path`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + fixture.edit(fixture.userDao, RoomAppFixture.USER_DAO.replace("ORDER BY name", "ORDER BY id")) + assertThat(analyzer.escalation(listOf(fixture.userDao))).isNotNull() + + fixture.edit(fixture.userDao, RoomAppFixture.USER_DAO) + assertThat(analyzer.escalation(listOf(fixture.userDao))).isNull() + } + + @Test + fun `hilt entry point on an activity live-reloads a body edit but not a constructor change`() { + val fixture = fixture() + val profile = AnnotationProcessorProfile.of(listOf("com.google.dagger:hilt-android-compiler:2.51")) + val hiltActivity = + fixture.write( + "HiltActivity.kt", + HILT_ACTIVITY, + ) + val baseline = AnnotationBaseline.capture(fixture.all + hiltActivity, profile) + val analyzer = AnnotationImpactAnalyzer(profile, baseline) + + fixture.edit(hiltActivity, HILT_ACTIVITY.replace("\"Hilt\"", "\"Hilt App\"")) + assertThat(analyzer.escalation(listOf(hiltActivity))).isNull() + + fixture.edit(hiltActivity, HILT_ACTIVITY.replace("val dao: UserDao", "val dao: UserDao, val converters: Converters")) + assertThat(analyzer.escalation(listOf(hiltActivity))).isNotNull() + } + + @Test + fun `the classifier routes a real Room edit set through the analyzer`() { + val fixture = fixture() + val classifier = ChangeClassifier(analyzer(fixture)) + fixture.edit(fixture.activity, RoomAppFixture.ACTIVITY.replace("\"Notes\"", "\"My Notes\"")) + + assertThat(classifier.classify(ChangedFiles.Known(setOf(fixture.activity)))) + .isEqualTo(BuildRoute.CodeOnly) + + fixture.edit(fixture.userDao, RoomAppFixture.USER_DAO.replace("ORDER BY name", "ORDER BY id")) + + assertThat(classifier.classify(ChangedFiles.Known(setOf(fixture.activity, fixture.userDao)))) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.ANNOTATION_PROCESSOR_INPUT_CHANGED)) + } + + /** + * A file the baseline knows but could not scan has no old facts to compare against, so + * nothing can prove the edit is safe. Per this class's asymmetry, unknown escalates. + */ + @Test + fun `a file whose baseline copy could not be scanned escalates`() { + val fixture = fixture() + // Unreadable only while capturing; the analyzer reads it fine, which is exactly the + // state that leaves a known file with null facts. + val baseline = + AnnotationBaseline.capture(fixture.all, roomProfile) { file -> + if (file == fixture.userDao) null else AnnotationBaseline.readOrNull(file) + } + val analyzer = AnnotationImpactAnalyzer(roomProfile, baseline) + + assertThat(analyzer.escalation(listOf(fixture.userDao))) + .isEqualTo("UserDao.kt: baseline copy could not be scanned") + } + + /** + * A file that is absent now and absent from the baseline never fed a processor, so it + * cannot have changed generated output. The deletion branch must not escalate on it. + */ + @Test + fun `deleting a file the baseline never saw does not escalate`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + val neverSeen = File(root, "app/src/main/java/com/example/notes/Absent.kt") + + assertThat(analyzer.escalation(listOf(neverSeen))).isNull() + } + + /** + * Stripping the processor annotations off a file changes what the processor generates + * just as much as adding them does - the generated DAO implementation has to go. + */ + @Test + fun `removing the processor annotations from a file escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + // Every Room annotation gone, so the file stops being a processor input entirely. + fixture.edit( + fixture.userDao, + """ + package com.example.notes + + interface UserDao { + fun all(): List + + fun insert(user: User) + } + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(fixture.userDao))) + .isEqualTo("UserDao.kt: processor-relevant annotations added or removed") + } + + /** + * An unreadable source at capture time is recorded as known-but-unscannable rather + * than skipped, and contributes no anchors - skipping it would make a later edit to it + * read as a file the baseline never saw, and silently pass. + */ + @Test + fun `capture records an unreadable source as known with no facts`() { + val fixture = fixture() + + val baseline = + AnnotationBaseline.capture(fixture.all, roomProfile) { file -> + if (file == fixture.userDao) null else AnnotationBaseline.readOrNull(file) + } + + assertThat(baseline.known(fixture.userDao)).isTrue() + assertThat(baseline.factsFor(fixture.userDao)).isNull() + // The other annotated files still contributed their anchors. + assertThat(baseline.anchorNames).isNotEmpty() + } + + private companion object { + val HILT_ACTIVITY = + """ + package com.example.notes + + import android.app.Activity + import android.os.Bundle + import dagger.hilt.android.AndroidEntryPoint + import javax.inject.Inject + + @AndroidEntryPoint + class HiltActivity( + @Inject val dao: UserDao, + ) : Activity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setTitle("Hilt") + } + } + """.trimIndent() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfileTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfileTest.kt new file mode 100644 index 0000000000..5f4ad3b8b0 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationProcessorProfileTest.kt @@ -0,0 +1,98 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** Which annotations a given processor set claims - the permissive/conservative switch. */ +class AnnotationProcessorProfileTest { + private fun factsOf(text: String) = SourceAnnotationScanner.scan(text)!! + + private fun isInput( + profile: AnnotationProcessorProfile, + text: String, + ): Boolean { + val facts = factsOf(text) + return facts.annotations.any { profile.isProcessorInput(it, facts) } + } + + private val room = AnnotationProcessorProfile.of(listOf("androidx.room:room-compiler:2.6.1")) + + @Test + fun `empty coordinates mean no processors`() { + assertThat(AnnotationProcessorProfile.of(emptyList()).hasProcessors).isFalse() + assertThat(AnnotationProcessorProfile.of(listOf(" ")).hasProcessors).isFalse() + } + + @Test + fun `room claims its own annotations`() { + assertThat(isInput(room, "import androidx.room.Entity\n@Entity\nclass User")).isTrue() + } + + @Test + fun `room does not claim a compose annotation`() { + assertThat( + isInput(room, "import androidx.compose.runtime.Composable\n@Composable\nfun Screen()"), + ).isFalse() + } + + @Test + fun `a qualified use site resolves without an import`() { + assertThat(isInput(room, "@androidx.room.Dao\ninterface UserDao")).isTrue() + } + + @Test + fun `an unresolvable name falls back to the processor vocabulary`() { + // No import at all: only the simple name is available. + assertThat(isInput(room, "@Dao\ninterface UserDao")).isTrue() + assertThat(isInput(room, "@Parcelize\nclass Thing")).isFalse() + } + + @Test + fun `a version catalog alias still identifies the processor`() { + val profile = AnnotationProcessorProfile.of(listOf("libs.room.compiler")) + assertThat(isInput(profile, "import androidx.room.Dao\n@Dao\ninterface UserDao")).isTrue() + } + + @Test + fun `an unrecognized processor claims every non language annotation`() { + val profile = AnnotationProcessorProfile.of(listOf("com.example:mystery:1.0")) + assertThat(isInput(profile, "import androidx.compose.runtime.Composable\n@Composable\nfun S()")).isTrue() + assertThat(isInput(profile, "@Whatever\nclass Thing")).isTrue() + } + + @Test + fun `an unrecognized processor still ignores language level annotations`() { + val profile = AnnotationProcessorProfile.of(listOf("com.example:mystery:1.0")) + assertThat(isInput(profile, "@Deprecated(\"x\")\nfun old()")).isFalse() + assertThat(isInput(profile, "@Suppress(\"UNCHECKED_CAST\")\nfun cast()")).isFalse() + assertThat(isInput(profile, "import java.lang.Override\n@Override\nfun go()")).isFalse() + } + + @Test + fun `mixing a recognized and an unrecognized processor stays conservative`() { + val profile = + AnnotationProcessorProfile.of( + listOf("androidx.room:room-compiler:2.6.1", "com.example:mystery:1.0"), + ) + assertThat(isInput(profile, "import androidx.compose.runtime.Composable\n@Composable\nfun S()")).isTrue() + } + + @Test + fun `hilt and dagger share a vocabulary`() { + val profile = AnnotationProcessorProfile.of(listOf("com.google.dagger:hilt-android-compiler:2.51")) + assertThat(isInput(profile, "import dagger.hilt.android.AndroidEntryPoint\n@AndroidEntryPoint\nclass A")).isTrue() + assertThat(isInput(profile, "import javax.inject.Inject\n@Inject\nlateinit var x: String")).isTrue() + assertThat(isInput(profile, "import androidx.room.Entity\n@Entity\nclass User")).isFalse() + } + + @Test + fun `moshi claims its json annotations`() { + val profile = AnnotationProcessorProfile.of(listOf("com.squareup.moshi:moshi-kotlin-codegen:1.15.0")) + assertThat(isInput(profile, "import com.squareup.moshi.JsonClass\n@JsonClass(generateAdapter = true)\nclass A")).isTrue() + } + + @Test + fun `no processors claims nothing at all`() { + assertThat(isInput(AnnotationProcessorProfile.NONE, "import androidx.room.Entity\n@Entity\nclass U")).isFalse() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/RoomAppFixture.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/RoomAppFixture.kt new file mode 100644 index 0000000000..e52b96ff4a --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/RoomAppFixture.kt @@ -0,0 +1,169 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import java.io.File + +/** + * A minimal Room + Hilt app materialized on disk, so the analyzer runs against real files. + * + * Covers the shapes that make annotation-aware classification hard: an `@Entity` with an + * un-annotated `@Embedded` value type, a non-annotated base class an entity inherits from, a + * `@TypeConverters` converter, `@Dao` SQL in annotation arguments, and plain UI files that + * must never rebaseline. + */ +class RoomAppFixture( + root: File, +) { + private val sourceDir = File(root, "app/src/main/java/com/example/notes").apply { mkdirs() } + + val user = write("User.kt", USER) + val address = write("Address.kt", ADDRESS) + val baseEntity = write("BaseEntity.kt", BASE_ENTITY) + val converters = write("Converters.kt", CONVERTERS) + val userDao = write("UserDao.kt", USER_DAO) + val database = write("AppDatabase.kt", DATABASE) + val viewModel = write("UserViewModel.kt", VIEW_MODEL) + val activity = write("MainActivity.kt", ACTIVITY) + val formatter = write("Formatter.kt", FORMATTER) + + val all: List = + listOf(user, address, baseEntity, converters, userDao, database, viewModel, activity, formatter) + + fun write( + name: String, + text: String, + ): File = File(sourceDir, name).apply { writeText(text) } + + /** Overwrites an existing fixture file, simulating a save. */ + fun edit( + file: File, + text: String, + ) { + file.writeText(text) + } + + companion object { + val USER = + """ + package com.example.notes + + import androidx.room.Embedded + import androidx.room.Entity + import androidx.room.PrimaryKey + + @Entity(tableName = "users") + data class User( + @PrimaryKey val id: Long, + val name: String, + @Embedded val address: Address, + ) : BaseEntity() + """.trimIndent() + + /** A plain data class an `@Embedded` property points at - no annotation of its own. */ + val ADDRESS = + """ + package com.example.notes + + data class Address( + val street: String, + val city: String, + ) + """.trimIndent() + + /** Room reads inherited fields; this base class has no annotation either. */ + val BASE_ENTITY = + """ + package com.example.notes + + abstract class BaseEntity { + var createdAt: Long = 0 + } + """.trimIndent() + + val CONVERTERS = + """ + package com.example.notes + + import androidx.room.TypeConverter + + class Converters { + @TypeConverter + fun fromTimestamp(value: Long?): String? { + return value?.toString() + } + } + """.trimIndent() + + val USER_DAO = + """ + package com.example.notes + + import androidx.room.Dao + import androidx.room.Insert + import androidx.room.Query + + @Dao + interface UserDao { + @Query("SELECT * FROM users ORDER BY name") + fun all(): List + + @Insert + fun insert(user: User) + } + """.trimIndent() + + val DATABASE = + """ + package com.example.notes + + import androidx.room.Database + import androidx.room.RoomDatabase + import androidx.room.TypeConverters + + @Database(entities = [User::class], version = 1) + @TypeConverters(Converters::class) + abstract class AppDatabase : RoomDatabase() { + abstract fun userDao(): UserDao + } + """.trimIndent() + + val VIEW_MODEL = + """ + package com.example.notes + + class UserViewModel( + private val dao: UserDao, + ) { + fun greeting(): String { + val count = dao.all().size + return "You have " + count + " users" + } + } + """.trimIndent() + + val ACTIVITY = + """ + package com.example.notes + + import android.app.Activity + import android.os.Bundle + + class MainActivity : Activity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setTitle("Notes") + } + } + """.trimIndent() + + val FORMATTER = + """ + package com.example.notes + + object Formatter { + fun format(name: String): String { + return name.trim() + } + } + """.trimIndent() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerEdgeTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerEdgeTest.kt new file mode 100644 index 0000000000..0f84ef7a2d --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerEdgeTest.kt @@ -0,0 +1,309 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** + * Structural edge cases of [SourceAnnotationScanner]: lexer bail-outs, `@` tokens that + * are not annotations, and body-exclusion shapes beyond the happy paths in + * [SourceAnnotationScannerTest]. The scanner's contract under stress is fail-safe: + * anything it cannot classify must either stay in the fingerprint or null the scan. + */ +class SourceAnnotationScannerEdgeTest { + @Test + fun `a file without a package declaration scans with an empty package`() { + val facts = SourceAnnotationScanner.scan("class NoPackage")!! + + assertThat(facts.packageName).isEmpty() + assertThat(facts.declaredTypeNames).containsExactly("NoPackage") + } + + @Test + fun `java static imports resolve to the imported path`() { + val facts = + SourceAnnotationScanner.scan( + """ + package com.example; + import static org.junit.Assert.assertEquals; + import java.util.List; + class J {} + """.trimIndent(), + )!! + + assertThat(facts.imports).containsExactly("org.junit.Assert.assertEquals", "java.util.List").inOrder() + } + + @Test + fun `an empty string literal at end of line does not open a raw string`() { + val facts = SourceAnnotationScanner.scan("""val a = ""${'\n'}val b = 2""")!! + + assertThat(facts.declarationFingerprint).contains("val b = 2") + } + + @Test + fun `a string ending in a bare escape at EOF bails`() { + assertThat(SourceAnnotationScanner.scan("""val s = "abc\""")).isNull() + } + + @Test + fun `a newline inside a single-quoted literal bails`() { + assertThat(SourceAnnotationScanner.scan("val s = \"abc\ndef\"")).isNull() + } + + @Test + fun `an escaped line break inside a single-quoted literal bails instead of throwing`() { + // The backslash masks the newline, so `code` keeps a line the mask lost and every later + // mask index is off by one. This used to throw ArrayIndexOutOfBoundsException out of a + // scope with no CoroutineExceptionHandler, i.e. it took CoGo down on any KSP project. + assertThat(SourceAnnotationScanner.scan("val s = \"a\\\nb\"")).isNull() + } + + @Test + fun `an escaped line break inside a char literal bails instead of throwing`() { + assertThat(SourceAnnotationScanner.scan("val c = '\\\n'")).isNull() + } + + @Test + fun `an escaped quote does not close the literal`() { + val facts = SourceAnnotationScanner.scan("""@Suppress("say \"hi\"") class A""")!! + + assertThat(facts.annotations.single().arguments).contains("\\\"hi\\\"") + } + + @Test + fun `a raw string closed with only two quotes at EOF bails`() { + assertThat(SourceAnnotationScanner.scan("val s = \"\"\"body\"\"")).isNull() + } + + @Test + fun `a close brace before any open bails`() { + assertThat(SourceAnnotationScanner.scan("}\nclass A {}")).isNull() + } + + @Test + fun `a qualified this reference is not an annotation`() { + val facts = + SourceAnnotationScanner.scan( + """ + class Outer { + val id = this@Outer.hashCode() + } + """.trimIndent(), + )!! + + assertThat(facts.annotations).isEmpty() + } + + @Test + fun `an at sign not followed by an identifier is skipped`() { + val facts = SourceAnnotationScanner.scan("val weird = 1 @ 2\nclass A")!! + + assertThat(facts.annotations).isEmpty() + assertThat(facts.declaredTypeNames).containsExactly("A") + } + + @Test + fun `an annotation at end of file parses without arguments`() { + val facts = SourceAnnotationScanner.scan("class A\n@Deprecated")!! + + assertThat(facts.annotations.single().name).isEqualTo("Deprecated") + assertThat(facts.annotations.single().arguments).isEmpty() + } + + @Test + fun `a fully qualified annotation keeps its dotted name`() { + val facts = SourceAnnotationScanner.scan("@java.lang.Deprecated class A")!! + + assertThat(facts.annotations.single().name).isEqualTo("java.lang.Deprecated") + } + + @Test + fun `a trailing dot after an annotation name belongs to the next token`() { + val facts = SourceAnnotationScanner.scan("class A\n@Outer.")!! + + assertThat(facts.annotations.single().name).isEqualTo("Outer") + } + + @Test + fun `an unclosed annotation argument list stops annotation extraction`() { + val facts = SourceAnnotationScanner.scan("@First class A\n@Broken(unclosed")!! + + // The paren never closes, so extraction keeps what it had - the file still + // scans (braces balance) and the earlier annotation survives. + assertThat(facts.annotations.map { it.name }).containsExactly("First") + } + + @Test + fun `tab-separated annotation arguments still attach`() { + val facts = SourceAnnotationScanner.scan("@Suppress\t(\"x\") class A")!! + + assertThat(facts.annotations.single().arguments).isEqualTo("""("x")""") + } + + @Test + fun `secondary constructor bodies are excluded from the fingerprint`() { + val facts = + SourceAnnotationScanner.scan( + """ + class A(val x: Int) { + constructor() : this(0) { + println("side effect") + } + } + """.trimIndent(), + )!! + + assertThat(facts.declarationFingerprint.joinToString("\n")).doesNotContain("side effect") + assertThat(facts.declarationFingerprint.joinToString("\n")).contains("constructor()") + } + + @Test + fun `init blocks are excluded from the fingerprint`() { + val facts = + SourceAnnotationScanner.scan( + """ + class A { + init { + val hidden = 1 + } + val kept = 2 + } + """.trimIndent(), + )!! + + val fingerprint = facts.declarationFingerprint.joinToString("\n") + assertThat(fingerprint).doesNotContain("hidden") + assertThat(fingerprint).contains("val kept = 2") + } + + @Test + fun `property accessor bodies are excluded from the fingerprint`() { + val facts = + SourceAnnotationScanner.scan( + """ + class A { + val v: Int + get() { + return 42 + } + } + """.trimIndent(), + )!! + + assertThat(facts.declarationFingerprint.joinToString("\n")).doesNotContain("return 42") + } + + @Test + fun `a single-line function keeps the fingerprint balanced`() { + // Opens and closes on one line: net zero braces, so the line itself stays. + val facts = + SourceAnnotationScanner.scan( + """ + class A { + fun f() { work() } + val kept = 1 + } + """.trimIndent(), + )!! + + assertThat(facts.declarationFingerprint.joinToString("\n")).contains("val kept = 1") + } + + @Test + fun `an empty string as the file's last token still scans`() { + val facts = SourceAnnotationScanner.scan("val s = \"\"")!! + + assertThat(facts.declarationFingerprint).contains("val s = \"\"") + } + + @Test + fun `an at sign as the file's last character is not an annotation`() { + val facts = SourceAnnotationScanner.scan("class A\n@")!! + + assertThat(facts.annotations).isEmpty() + } + + @Test + fun `at signs glued to identifiers or other at signs are not annotations`() { + val facts = SourceAnnotationScanner.scan("val a = b@c\nval d_@e = 1\nval f = g@@h\nclass A")!! + + assertThat(facts.annotations).isEmpty() + } + + @Test + fun `a lone double quote inside a raw string does not close it`() { + val facts = SourceAnnotationScanner.scan("val s = \"\"\"say \" once\"\"\"\nval kept = 1")!! + + assertThat(facts.declarationFingerprint.joinToString("\n")).contains("val kept = 1") + } + + @Test + fun `a block comment on a single line strips without eating the line`() { + val facts = SourceAnnotationScanner.scan("val a = /* inline */ 1")!! + + assertThat(facts.declarationFingerprint).containsExactly("val a = 1") + } + + @Test + fun `a line comment as the file's last bytes strips cleanly`() { + val facts = SourceAnnotationScanner.scan("class A // no trailing newline")!! + + assertThat(facts.declarationFingerprint).containsExactly("class A") + } + + @Test + fun `a lone star inside a block comment does not close it`() { + val facts = SourceAnnotationScanner.scan("val a = /* 2*3 */ 6")!! + + assertThat(facts.declarationFingerprint).containsExactly("val a = 6") + } + + @Test + fun `a lambda default in the signature still excludes only the body`() { + // Two opens on the signature line ({} default + the body brace): the body mark + // must attach to the LAST open, not the first. + val facts = + SourceAnnotationScanner.scan( + """ + class A { + fun f(block: () -> Unit = {}) { + hiddenWork() + } + val kept = 1 + } + """.trimIndent(), + )!! + + val fingerprint = facts.declarationFingerprint.joinToString("\n") + assertThat(fingerprint).doesNotContain("hiddenWork") + assertThat(fingerprint).contains("val kept = 1") + } + + @Test + fun `a multi-line raw string keeps its line structure and its braces masked`() { + val facts = + SourceAnnotationScanner.scan( + "class A {\n\tval sql = \"\"\"SELECT *\n\t\tFROM { nowhere }\n\t\"\"\"\n}", + )!! + + // The brace inside the raw string must not have derailed nesting, and the + // literal's content stays in the fingerprint verbatim. + assertThat(facts.declarationFingerprint.joinToString("\n")).contains("FROM { nowhere }") + } + + @Test + fun `division and multiplication are not comment openers`() { + val facts = SourceAnnotationScanner.scan("val half = 6 / 2\nval product = 2 * 3")!! + + assertThat(facts.declarationFingerprint) + .containsExactly("val half = 6 / 2", "val product = 2 * 3") + .inOrder() + } + + @Test + fun `annotation argument types count as referenced`() { + val facts = SourceAnnotationScanner.scan("@TypeConverters(DateConverter::class) class Db")!! + + assertThat(facts.referencedTypeNames).contains("DateConverter") + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerTest.kt new file mode 100644 index 0000000000..4ff163c2d8 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScannerTest.kt @@ -0,0 +1,219 @@ +package org.appdevforall.cotg.quickbuild.domain.annotations + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** + * The scanner's two jobs: find every annotation with its arguments verbatim, and produce + * a declaration fingerprint that moves when declarations move and holds still when only + * a body, a comment or whitespace moves. + */ +class SourceAnnotationScannerTest { + private fun scan(text: String) = SourceAnnotationScanner.scan(text) + + @Test + fun `finds annotations with arguments and resolves package and imports`() { + val facts = + scan( + """ + package com.example + + import androidx.room.Entity + import androidx.room.PrimaryKey + + @Entity(tableName = "users") + data class User(@PrimaryKey val id: Long) + """.trimIndent(), + )!! + + assertThat(facts.packageName).isEqualTo("com.example") + assertThat(facts.imports).containsExactly("androidx.room.Entity", "androidx.room.PrimaryKey") + assertThat(facts.annotations.map { it.name }).containsExactly("Entity", "PrimaryKey").inOrder() + assertThat(facts.annotations.first().arguments).isEqualTo("(tableName = \"users\")") + assertThat(facts.declaredTypeNames).containsExactly("User") + } + + @Test + fun `keeps annotation string arguments verbatim`() { + val sql = "@Query(\"SELECT * FROM users WHERE id = :id\")\nfun byId(id: Long): User" + assertThat(scan(sql)!!.annotations.single().arguments) + .isEqualTo("(\"SELECT * FROM users WHERE id = :id\")") + } + + @Test + fun `keeps nested parentheses in annotation arguments`() { + val text = "@Entity(indices = [Index(value = [\"name\"])])\nclass User" + assertThat(scan(text)!!.annotations.single().arguments) + .isEqualTo("(indices = [Index(value = [\"name\"])])") + } + + @Test + fun `keeps kotlin use-site targets distinct`() { + assertThat(scan("@field:Json(name = \"a\") val a: String")!!.annotations.single().name) + .isEqualTo("Json") + } + + @Test + fun `ignores an at sign inside a string literal`() { + assertThat(scan("val email = \"nobody@example.com\"")!!.annotations).isEmpty() + } + + @Test + fun `ignores annotations inside comments`() { + val text = + """ + // @Entity + /* @Dao */ + class Plain + """.trimIndent() + assertThat(scan(text)!!.annotations).isEmpty() + } + + @Test + fun `fingerprint ignores comments and whitespace`() { + val a = + """ + class A { + val x: Int = 1 + } + """.trimIndent() + val b = + """ + // leading note + class A { + /* about x */ + val x: Int = 1 + } + """.trimIndent() + + assertThat(scan(a)!!.declarationFingerprint).isEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `fingerprint ignores function bodies`() { + val a = + """ + class A { + fun go(): Int { + return 1 + } + } + """.trimIndent() + val b = + """ + class A { + fun go(): Int { + val doubled = 2 * 21 + return doubled + } + } + """.trimIndent() + + assertThat(scan(a)!!.declarationFingerprint).isEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `fingerprint moves when a declaration moves`() { + val a = "class A {\n\tval x: Int = 1\n}" + val b = "class A {\n\tval x: Long = 1\n}" + + assertThat(scan(a)!!.declarationFingerprint).isNotEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `fingerprint keeps a nested class body`() { + val a = "class A {\n\tclass Inner {\n\t\tval x: Int = 1\n\t}\n}" + val b = "class A {\n\tclass Inner {\n\t\tval x: Long = 1\n\t}\n}" + + assertThat(scan(a)!!.declarationFingerprint).isNotEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `fingerprint keeps a property initializer lambda`() { + val a = "class A {\n\tval x = lazy {\n\t\t1\n\t}\n}" + val b = "class A {\n\tval x = lazy {\n\t\t2\n\t}\n}" + + // Not a function signature, so the block is NOT treated as a body - conservative. + assertThat(scan(a)!!.declarationFingerprint).isNotEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `braces inside string literals do not confuse nesting`() { + val facts = scan("class A {\n\tfun go(): String {\n\t\treturn \"{{{\"\n\t}\n}") + assertThat(facts).isNotNull() + } + + @Test + fun `braces inside a raw string do not confuse nesting`() { + val facts = scan("class A {\n\tval q = \"\"\"{ \"a\": 1 }\"\"\"\n}") + assertThat(facts).isNotNull() + } + + @Test + fun `char literal brace does not confuse nesting`() { + assertThat(scan("class A {\n\tval c = '{'\n}")).isNotNull() + } + + @Test + fun `unbalanced braces bail`() { + assertThat(scan("class A {\n\tval x = 1\n")).isNull() + } + + @Test + fun `unterminated block comment bails`() { + assertThat(scan("class A {}\n/* still going")).isNull() + } + + @Test + fun `unterminated raw string bails`() { + assertThat(scan("val q = \"\"\"open")).isNull() + } + + @Test + fun `java method bodies are excluded from the fingerprint`() { + val a = + """ + package com.example; + + public class A { + public int go() { + return 1; + } + } + """.trimIndent() + val b = + """ + package com.example; + + public class A { + public int go() { + int x = 21 * 2; + return x; + } + } + """.trimIndent() + + assertThat(scan(a)!!.declarationFingerprint).isEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `java field change moves the fingerprint`() { + val a = "public class A {\n\tint x = 1;\n}" + val b = "public class A {\n\tlong x = 1;\n}" + + assertThat(scan(a)!!.declarationFingerprint).isNotEqualTo(scan(b)!!.declarationFingerprint) + } + + @Test + fun `records referenced type names from declarations and annotation arguments`() { + val facts = + scan( + """ + @Database(entities = [User::class], version = 1) + abstract class AppDatabase : RoomDatabase() + """.trimIndent(), + )!! + + assertThat(facts.referencedTypeNames).containsAtLeast("User", "RoomDatabase", "AppDatabase") + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRouteTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRouteTest.kt new file mode 100644 index 0000000000..9406ea6b45 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/BuildRouteTest.kt @@ -0,0 +1,42 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** + * The [recompilesCode] classification, one case per route. + * + * Its only production caller gates the stale-classes guard (`QuickBuildSessionManager`: + * `if (event.route is BuildRoute.WarmCompile || !event.route.recompilesCode) return`), so a + * route on the wrong side silently skips a guard that should run or runs one that should not. + */ +class BuildRouteTest { + @Test + fun `every route that produces class files reports recompilesCode`() { + assertThat(BuildRoute.CodeOnly.recompilesCode).isTrue() + assertThat(BuildRoute.CodeAndResources.recompilesCode).isTrue() + // NoOp still runs the compiler - it is "compiled and nothing moved", not "skipped". + assertThat(BuildRoute.NoOp.recompilesCode).isTrue() + // WarmCompile recompiles but never deploys; callers reasoning about the running + // app must exclude it separately, which is why it is true here. + assertThat(BuildRoute.WarmCompile.recompilesCode).isTrue() + } + + @Test + fun `routes that move no class file do not report recompilesCode`() { + assertThat(BuildRoute.ResourcesOnly.recompilesCode).isFalse() + assertThat(BuildRoute.AssetsOnly.recompilesCode).isFalse() + assertThat(BuildRoute.FullGradleBuild(InvalidationReason.MANIFEST_CHANGED).recompilesCode).isFalse() + } + + /** + * The classification is a property of the route alone: a full Gradle build hands the + * whole job to Gradle whatever invalidated the baseline, so no reason may flip it. + */ + @Test + fun `no invalidation reason makes a full gradle build recompile on the live path`() { + InvalidationReason.entries.forEach { reason -> + assertThat(BuildRoute.FullGradleBuild(reason).recompilesCode).isFalse() + } + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierEdgeTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierEdgeTest.kt new file mode 100644 index 0000000000..12996c202a --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierEdgeTest.kt @@ -0,0 +1,78 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +import com.google.common.truth.Truth.assertThat +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.junit.jupiter.api.Test +import java.io.File + +/** + * Negative sides of [ChangeClassifier]'s Gradle-config detection: files whose NAMES + * look configuration-ish but whose paths say otherwise must not trip a full Gradle + * invalidation - a spurious rebaseline costs the user a ~97 s proxy app rebuild. + */ +class ChangeClassifierEdgeTest { + private val classifier = ChangeClassifier() + + private fun classify(vararg paths: String): BuildRoute = classifier.classify(ChangedFiles.Known(paths.map(::File).toSet())) + + @Test + fun `a toml outside any gradle segment is not gradle config`() { + // e.g. a Rust/Cargo file vendored under src: unsupported shape, honest fallback - + // but NOT because it was mistaken for a version catalog. + assertThat(classify("app/src/main/java/com/example/Cargo.toml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a wrapper-named properties file outside the wrapper dir is not gradle config`() { + assertThat(classify("app/src/main/assets/gradle-wrapper.properties")) + .isEqualTo(BuildRoute.AssetsOnly) + } + + @Test + fun `a properties file inside the wrapper dir with another name is not gradle config`() { + assertThat(classify("gradle/wrapper/other.properties")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a res-like path outside src is not a resource`() { + // A stray res/ dir at the project root is not an Android source-set resource. + assertThat(classify("res/values/strings.xml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a kotlin file in a package named res is code, not a resource`() { + // `res` is a legal package name. Read as a resource, the route becomes ResourcesOnly: + // aapt2 relinks, the cached dex is reused, nothing compiles, and the edit is silently + // missing from the running app. + assertThat(classify("app/src/main/java/com/example/res/Strings.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `a package named after a source set stays on the quick path`() { + // `test` is a legal package name. The source-set guard is anchored at the src child, so + // a nested package cannot rename the source set the file is really in - reading this as + // src/test would push every edit under it to a needless full Gradle build. + assertThat(classify("app/src/main/java/com/example/test/Helper.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `a java file in a package named assets is code, not an asset`() { + assertThat(classify("app/src/main/java/com/example/assets/Loader.java")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `namesResource is false for a code file in a package named res`() { + // Same shape via the diagnostic-attribution helper: a kotlinc error in this package must + // not be blamed on aapt2, and must not count toward the stuck-relink escalation. + assertThat(ChangeClassifier.namesResource(File("app/src/main/java/com/example/res/Strings.kt"))) + .isFalse() + assertThat(ChangeClassifier.namesResource(File("app/src/main/res/values/strings.xml"))) + .isTrue() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierTest.kt new file mode 100644 index 0000000000..ec8603088a --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/ChangeClassifierTest.kt @@ -0,0 +1,411 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +import com.google.common.truth.Truth.assertThat +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.appdevforall.cotg.quickbuild.domain.annotations.AnnotationImpact +import org.junit.jupiter.api.Test +import java.io.File + +/** + * One test per edit class [ChangeClassifier] routes, plus the precedence and + * honesty-fallback rules. + */ +class ChangeClassifierTest { + private val classifier = ChangeClassifier() + + private fun classify(vararg paths: String): BuildRoute = classifier.classify(ChangedFiles.Known(paths.map(::File).toSet())) + + @Test + fun `kotlin source is code only`() { + assertThat(classify("app/src/main/java/com/example/Main.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `java source is code only`() { + assertThat(classify("app/src/main/java/com/example/Main.java")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `resource value file is resources only`() { + assertThat(classify("app/src/main/res/values/strings.xml")) + .isEqualTo(BuildRoute.ResourcesOnly) + } + + @Test + fun `layout and drawable files are resources only`() { + assertThat( + classify( + "app/src/main/res/layout/activity_main.xml", + "app/src/main/res/drawable/icon.png", + ), + ).isEqualTo(BuildRoute.ResourcesOnly) + } + + @Test + fun `asset file is assets only`() { + assertThat(classify("app/src/main/assets/data/levels.json")) + .isEqualTo(BuildRoute.AssetsOnly) + } + + @Test + fun `mixed kotlin and resource save compiles AND relinks`() { + assertThat( + classify( + "app/src/main/java/com/example/Main.kt", + "app/src/main/res/values/strings.xml", + ), + ).isEqualTo(BuildRoute.CodeAndResources) + } + + @Test + fun `code with assets classifies as code only`() { + // Assets ride along in the deploy payload regardless; compile is the driver. + assertThat( + classify( + "app/src/main/java/com/example/Main.kt", + "app/src/main/assets/data/levels.json", + ), + ).isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `manifest change invalidates the session`() { + assertThat(classify("app/src/main/AndroidManifest.xml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.MANIFEST_CHANGED)) + } + + @Test + fun `gradle build file invalidates the session`() { + assertThat(classify("app/build.gradle.kts")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED)) + assertThat(classify("settings.gradle")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED)) + assertThat(classify("gradle.properties")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED)) + } + + @Test + fun `version catalog and wrapper properties invalidate the session`() { + assertThat(classify("gradle/libs.versions.toml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED)) + assertThat(classify("gradle/wrapper/gradle-wrapper.properties")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED)) + } + + @Test + fun `invalidation wins over any accompanying code change`() { + assertThat( + classify( + "app/src/main/java/com/example/Main.kt", + "app/src/main/AndroidManifest.xml", + ), + ).isInstanceOf(BuildRoute.FullGradleBuild::class.java) + } + + @Test + fun `unsupported file under src falls back honestly`() { + // A java-resource the quick path can't package: serving a quick build would be + // stale, so it must route to Gradle. + assertThat(classify("app/src/main/resources/config.properties")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `native library under jniLibs falls back honestly`() { + // The quick path has no relink/redeploy story for a changed .so - serving a build that + // still has the OLD native library loaded would be silently stale, so this must route + // to Gradle like any other unsupported-file change (a native app's .c/.h sources). + assertThat(classify("app/src/main/jniLibs/arm64-v8a/libnativestub.so")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `unknown changed-set forces a full quick recompile, not a Gradle fallback`() { + assertThat(classifier.classify(ChangedFiles.Unknown)) + .isEqualTo(BuildRoute.CodeAndResources) + } + + @Test + fun `empty known set is a no-op`() { + assertThat(classifier.classify(ChangedFiles.Known.EMPTY)).isEqualTo(BuildRoute.NoOp) + } + + @Test + fun `annotation impact escalates a code change to a Gradle rebaseline`() { + assertThat( + classifierWith(active = true, escalates = true) + .classify(ChangedFiles.Known(setOf(File("app/src/main/java/Dao.kt")))), + ).isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.ANNOTATION_PROCESSOR_INPUT_CHANGED)) + } + + @Test + fun `annotation impact leaves a safe code change on the live reload path`() { + assertThat( + classifierWith(active = true, escalates = false) + .classify(ChangedFiles.Known(setOf(File("app/src/main/java/Ui.kt")))), + ).isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `annotation impact is never consulted for a resource-only change`() { + var consulted = false + val impact = + object : AnnotationImpact { + override val active = true + + override fun escalation(changedCodeFiles: List): String { + consulted = true + return "should not be reached" + } + } + + assertThat( + ChangeClassifier(impact) + .classify(ChangedFiles.Known(setOf(File("app/src/main/res/values/strings.xml")))), + ).isEqualTo(BuildRoute.ResourcesOnly) + assertThat(consulted).isFalse() + } + + @Test + fun `an unknown changed-set falls back to Gradle when processors are configured`() { + // Cannot enumerate what changed, so cannot prove it missed processor input. + assertThat(classifierWith(active = true, escalates = false).classify(ChangedFiles.Unknown)) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.ANNOTATION_PROCESSOR_INPUT_CHANGED)) + } + + @Test + fun `hasRecognizedShape is true for every classifiable kind and false for unsupported`() { + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/java/com/example/Main.kt"))) + .isTrue() + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/java/com/example/Main.java"))) + .isTrue() + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/res/values/strings.xml"))) + .isTrue() + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/assets/data/levels.json"))) + .isTrue() + assertThat(ChangeClassifier.hasRecognizedShape(File("app/build.gradle.kts"))).isTrue() + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/AndroidManifest.xml"))) + .isTrue() + // The sibling temp an atomic-rename save leaves behind: no dot-prefix or known + // suffix, no extension at all - exactly the shape WatchFilter can't name-filter. + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/java/com/example/sedAbC123"))) + .isFalse() + assertThat(ChangeClassifier.hasRecognizedShape(File("app/src/main/resources/config.properties"))) + .isFalse() + } + + private fun classifyRemoved(vararg paths: String): BuildRoute = + classifier.classify(ChangedFiles.Known(emptySet(), paths.map(::File).toSet())) + + @Test + fun `a removed kotlin source is code only`() { + assertThat(classifyRemoved("app/src/main/java/com/example/Main.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `a removed resource is resources only`() { + assertThat(classifyRemoved("app/src/main/res/values/strings.xml")) + .isEqualTo(BuildRoute.ResourcesOnly) + } + + @Test + fun `a removed gradle file is a full gradle build`() { + assertThat(classifyRemoved("app/build.gradle.kts")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.GRADLE_CONFIG_CHANGED)) + } + + @Test + fun `a removed manifest is a full gradle build`() { + assertThat(classifyRemoved("app/src/main/AndroidManifest.xml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.MANIFEST_CHANGED)) + } + + @Test + fun `a modified source plus a removed source is one code build`() { + assertThat( + classifier.classify( + ChangedFiles.Known( + files = setOf(File("app/src/main/java/com/example/A.kt")), + removed = setOf(File("app/src/main/java/com/example/B.kt")), + ), + ), + ).isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `an empty known set with no removals is a no-op`() { + assertThat(classifier.classify(ChangedFiles.Known.EMPTY)).isEqualTo(BuildRoute.NoOp) + } + + // Multi-module boundary (Level 1): a live reload builds only the app module. + + private val moduleAware = ChangeClassifier(fastPathRoots = listOf(File("app/src"))) + + private fun classifyScoped(vararg paths: String): BuildRoute = moduleAware.classify(ChangedFiles.Known(paths.map(::File).toSet())) + + @Test + fun `app-module code inside the live-reload scope stays a code build`() { + assertThat(classifyScoped("app/src/main/java/com/example/A.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + @Test + fun `library-module code outside the scope rebaselines`() { + assertThat(classifyScoped("feature-login/src/main/java/com/example/Login.kt")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED)) + } + + @Test + fun `library-module resource outside the scope rebaselines`() { + assertThat(classifyScoped("core-ui/src/main/res/values/colors.xml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED)) + } + + @Test + fun `library-module asset outside the scope rebaselines`() { + assertThat(classifyScoped("data/src/main/assets/seed.json")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED)) + } + + @Test + fun `an app edit beside a library edit rebaselines - never live-reload a partial changeset`() { + assertThat( + classifyScoped( + "app/src/main/java/com/example/A.kt", + "feature-login/src/main/java/com/example/Login.kt", + ), + ).isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED)) + } + + @Test + fun `a removed library-module source rebaselines`() { + assertThat( + moduleAware.classify( + ChangedFiles.Known(files = emptySet(), removed = setOf(File("feature-login/src/main/java/com/example/Gone.kt"))), + ), + ).isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.NON_APP_MODULE_SOURCE_CHANGED)) + } + + @Test + fun `empty live-reload roots disables the boundary - single-module behavior is unchanged`() { + // The default classifier (no fastPathRoots) must treat any src code as a code build, + // preserving pre-multi-module semantics for single-module projects and shape tests. + assertThat(classify("feature-login/src/main/java/com/example/Login.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + // Source sets (Level 1, second axis): only src/main is on the quick path. + + @Test + fun `a unit test source is not on the quick path`() { + // allSources() reads src/main only, so the daemon would compile nothing and the deploy + // would still claim a reload. + assertThat(classifyScoped("app/src/test/java/com/example/ATest.kt")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `an instrumentation test source is not on the quick path`() { + assertThat(classifyScoped("app/src/androidTest/java/com/example/AUiTest.kt")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a debug source set is not on the quick path`() { + // Unlike a test source this IS part of the app, so serving a quick build would ship + // the running app without the edit. + assertThat(classifyScoped("app/src/debug/java/com/example/DebugOnly.kt")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a flavor source set is not on the quick path`() { + assertThat(classifyScoped("app/src/free/java/com/example/FreeOnly.kt")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a non-main source set resource is not on the quick path`() { + // The res half of the same hole: resDirs() is src/main/res alone, so a relink would + // silently drop this overlay. A code-only fix would miss this. + assertThat(classifyScoped("app/src/debug/res/values/strings.xml")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a non-main source set asset is not on the quick path`() { + assertThat(classifyScoped("app/src/debug/assets/seed.json")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a main source beside a debug source rebaselines`() { + assertThat( + classifyScoped( + "app/src/main/java/com/example/A.kt", + "app/src/debug/java/com/example/DebugOnly.kt", + ), + ).isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + // Assets below API 30: nothing on the device serves a deployed asset payload. + + private val noAssetServing = ChangeClassifier(assetsLiveReloadable = false) + + private fun classifyUnservedAssets(vararg paths: String): BuildRoute = + noAssetServing.classify(ChangedFiles.Known(paths.map(::File).toSet())) + + @Test + fun `an asset edit rebaselines when the device cannot serve deployed assets`() { + assertThat(classifyUnservedAssets("app/src/main/assets/data/levels.json")) + .isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `code beside an asset rebaselines too - the asset rides the code payload`() { + assertThat( + classifyUnservedAssets( + "app/src/main/java/com/example/Main.kt", + "app/src/main/assets/data/levels.json", + ), + ).isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `a removed asset rebaselines when the device cannot serve deployed assets`() { + assertThat( + noAssetServing.classify( + ChangedFiles.Known(emptySet(), setOf(File("app/src/main/assets/data/levels.json"))), + ), + ).isEqualTo(BuildRoute.FullGradleBuild(InvalidationReason.UNSUPPORTED_FILE_CHANGED)) + } + + @Test + fun `the gate is assets-only - resources keep their own legacy path`() { + // API 28/29 resources DO have a swap mechanism (LegacyResourceSwap), so gating them + // here would send every strings-xml edit to Gradle for nothing. + assertThat(classifyUnservedAssets("app/src/main/res/values/strings.xml")) + .isEqualTo(BuildRoute.ResourcesOnly) + } + + @Test + fun `code with no asset stays on the live reload path when assets cannot be served`() { + assertThat(classifyUnservedAssets("app/src/main/java/com/example/Main.kt")) + .isEqualTo(BuildRoute.CodeOnly) + } + + private fun classifierWith( + active: Boolean, + escalates: Boolean, + ): ChangeClassifier = + ChangeClassifier( + object : AnnotationImpact { + override val active = active + + override fun escalation(changedCodeFiles: List): String? = "annotation input changed".takeIf { escalates } + }, + ) +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilterTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilterTest.kt new file mode 100644 index 0000000000..ba18929170 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/classify/TestSourceFilterTest.kt @@ -0,0 +1,178 @@ +package org.appdevforall.cotg.quickbuild.domain.classify + +import com.google.common.truth.Truth.assertThat +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.junit.jupiter.api.Test +import java.io.File + +/** + * The boundary between a save Quick Build ignores and one it must still build. + * + * The precision matters in both directions. Ignoring a source set that DOES ship in the variant + * (`src/debug`, a flavor) would leave the running app missing an edit the user made, with no + * warning; building a test source set costs a full Gradle build that cannot change the app at all. + */ +class TestSourceFilterTest { + private fun split( + modified: List = emptyList(), + removed: List = emptyList(), + ): TestSourceFilter.Split = + TestSourceFilter.split( + ChangedFiles.Known(modified.map(::File).toSet(), removed.map(::File).toSet()), + ) + + @Test + fun `a unit test save is dropped and leaves nothing to build`() { + val split = split(modified = listOf("app/src/test/java/com/example/FooTest.kt")) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `an instrumentation test save is dropped`() { + val split = split(modified = listOf("app/src/androidTest/java/com/example/FooTest.kt")) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `testFixtures is dropped - it ships to consumers' tests, not into the app`() { + val split = split(modified = listOf("app/src/testFixtures/java/com/example/Fixtures.kt")) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `a flavor-qualified test source set is dropped`() { + // AGP appends the flavor and build type: src/testProDebug, src/androidTestDebug. + val split = + split( + modified = + listOf( + "app/src/testProDebug/java/com/example/FooTest.kt", + "app/src/androidTestDebug/java/com/example/BarTest.kt", + ), + ) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `a test resource or asset is dropped too, not just test code`() { + val split = + split( + modified = + listOf( + "app/src/test/resources/fixture.json", + "app/src/androidTest/assets/sample.png", + ), + ) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `a deleted test file is dropped as well - removing a test deploys no more than saving one`() { + val split = split(removed = listOf("app/src/test/java/com/example/FooTest.kt")) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `src debug ships in the variant, so it is kept`() { + // The precision this class exists for: a debug source set IS compiled into the app the + // user runs. Ignoring it would leave the running app missing their edit, silently. + val debugSource = "app/src/debug/java/com/example/Debug.kt" + + val split = split(modified = listOf(debugSource)) + + assertThat(split.droppedTestSources).isFalse() + assertThat(split.buildable.files).containsExactly(File(debugSource)) + } + + @Test + fun `a flavor source set is kept`() { + val flavorSource = "app/src/pro/java/com/example/Pro.kt" + + val split = split(modified = listOf(flavorSource)) + + assertThat(split.droppedTestSources).isFalse() + assertThat(split.buildable.files).containsExactly(File(flavorSource)) + } + + @Test + fun `a flavor whose name merely begins with test is kept`() { + // "testflavor" is a shipping source set. A bare startsWith("test") would ignore every + // save in it, and the user would never be told why their edit did not appear. + val flavorSource = "app/src/testflavor/java/com/example/Thing.kt" + + val split = split(modified = listOf(flavorSource)) + + assertThat(split.droppedTestSources).isFalse() + assertThat(split.buildable.files).containsExactly(File(flavorSource)) + } + + @Test + fun `a package named test in main is kept`() { + // The source set is the innermost src child, so a package called `test` cannot rename + // the source set the file is really in. + val mainSource = "app/src/main/java/com/example/test/Helper.kt" + + val split = split(modified = listOf(mainSource)) + + assertThat(split.droppedTestSources).isFalse() + assertThat(split.buildable.files).containsExactly(File(mainSource)) + } + + @Test + fun `a main save beside a test save still builds, and the drop is still reported`() { + val mainSource = "app/src/main/java/com/example/Foo.kt" + + val split = + split(modified = listOf(mainSource, "app/src/test/java/com/example/FooTest.kt")) + + // The main edit must reach the build - a save-all writes both at once, and dropping the + // whole batch would silently strand the edit the user can actually see. + assertThat(split.buildable.files).containsExactly(File(mainSource)) + // And the notice is still owed: the test half did not deploy either. + assertThat(split.droppedTestSources).isTrue() + } + + @Test + fun `a batch with no test source is passed through untouched`() { + val mainSource = "app/src/main/java/com/example/Foo.kt" + val removedSource = "app/src/main/java/com/example/Bar.kt" + + val split = split(modified = listOf(mainSource), removed = listOf(removedSource)) + + assertThat(split.droppedTestSources).isFalse() + assertThat(split.buildable.files).containsExactly(File(mainSource)) + assertThat(split.buildable.removed).containsExactly(File(removedSource)) + } + + @Test + fun `a test source in a library module is dropped as well`() { + // The module makes no difference: nothing in a test source set anywhere reaches the + // app Quick Build deploys. + val split = split(modified = listOf("lib/src/test/java/com/example/LibTest.kt")) + + assertThat(split.droppedTestSources).isTrue() + assertThat(split.buildable.isEmpty).isTrue() + } + + @Test + fun `a path with no source set at all is kept`() { + val gradleFile = "app/build.gradle.kts" + + val split = split(modified = listOf(gradleFile)) + + assertThat(split.droppedTestSources).isFalse() + assertThat(split.buildable.files).containsExactly(File(gradleFile)) + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingEdgeTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingEdgeTest.kt new file mode 100644 index 0000000000..bc9687d1df --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingEdgeTest.kt @@ -0,0 +1,46 @@ +@file:OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) + +package org.appdevforall.cotg.quickbuild.domain.watch + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import java.io.File + +/** + * The completion-flush guarantee of [coalesceChanges]: a watcher stream that ends + * mid-batch (session teardown) must still deliver the pending batch instead of + * dropping it - the never-stale invariant's last line. + */ +class ChangeCoalescingEdgeTest { + private fun f(name: String) = File("/proj/app/src/main/java/$name") + + @Test + fun `upstream completion flushes the pending batch without waiting for the quiet window`() = + runTest { + val batches = + flowOf( + WatchEvent.Modified(f("A.kt")), + WatchEvent.Removed(f("B.kt")), + ).coalesceChanges(quietMillis = 60_000, maxMillis = 600_000).toList() + + // Both timers are still armed (their windows are enormous); only the + // upstream's completion can have delivered this batch. + assertThat(batches).hasSize(1) + assertThat(batches[0].files).containsExactly(f("A.kt")) + assertThat(batches[0].removed).containsExactly(f("B.kt")) + } + + @Test + fun `an empty upstream completes with no batch`() = + runTest { + val batches = + flowOf() + .coalesceChanges(quietMillis = 10, maxMillis = 100) + .toList() + + assertThat(batches).isEmpty() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingTest.kt new file mode 100644 index 0000000000..478eb6560a --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/ChangeCoalescingTest.kt @@ -0,0 +1,220 @@ +@file:OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) + +package org.appdevforall.cotg.quickbuild.domain.watch + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.buffer +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.test.runTest +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.junit.jupiter.api.Test +import java.io.File + +/** + * Pins the debounce policy: a trailing [quietMillis] window, reset on every event, capped + * at [maxMillis] since the batch's first event. Virtual-time tests so they are + * deterministic and instant. Batches carry both the modified/created paths and the removed + * ones. + */ +class ChangeCoalescingTest { + private fun f(name: String) = File("/proj/app/src/main/java/$name") + + private fun m(name: String): WatchEvent = WatchEvent.Modified(f(name)) + + private fun d(name: String): WatchEvent = WatchEvent.Removed(f(name)) + + @Test + fun `a single change emits one batch after the quiet window`() = + runTest { + val batches = + flowOf(m("A.kt")).coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(1) + assertThat(batches.single().files).containsExactly(f("A.kt")) + assertThat(batches.single().removed).isEmpty() + } + + @Test + fun `writes within the quiet window coalesce into one batch`() = + runTest { + val source = + flow { + emit(m("A.kt")) + delay(50) + emit(m("B.kt")) + delay(50) + emit(m("C.kt")) + } + + val batches = source.coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(1) + assertThat(batches.single().files).containsExactly(f("A.kt"), f("B.kt"), f("C.kt")) + } + + @Test + fun `a gap longer than the quiet window splits into two batches`() = + runTest { + val source = + flow { + emit(m("A.kt")) + delay(300) // > quiet window: batch 1 flushes + emit(m("B.kt")) + } + + val batches = source.coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(2) + assertThat(batches[0].files).containsExactly(f("A.kt")) + assertThat(batches[1].files).containsExactly(f("B.kt")) + } + + @Test + fun `a continuous stream is capped and flushes at maxMillis`() = + runTest { + // An event every 120 ms (< 150 quiet) for 1.8 s: the quiet timer keeps resetting, + // so only the cap can flush. 120 rather than 100 so no event falls on maxMillis + // itself, which would leave the batch it lands in decided by timer tie-breaking. + val source = + flow { + repeat(15) { i -> + emit(m("S$i.kt")) + delay(120) + } + } + + val batches = mutableListOf() + val flushedAt = mutableListOf() + source.coalesceChanges(quietMillis = 150, maxMillis = 1000).collect { batch -> + batches.add(batch) + flushedAt.add(testScheduler.currentTime) + } + + // The cap - not the end of the stream - produces the first batch: it lands at + // maxMillis carrying everything written by then, and the stragglers wait for the + // terminal flush. Assert the two batches separately rather than as a union, so a + // path duplicated into both (or dropped from one) cannot hide. + assertThat(batches).hasSize(2) + assertThat(flushedAt[0]).isEqualTo(1000L) + assertThat(batches[0].files).containsExactlyElementsIn((0..8).map { f("S$it.kt") }) + assertThat(flushedAt[1]).isEqualTo(1800L) + assertThat(batches[1].files).containsExactlyElementsIn((9..14).map { f("S$it.kt") }) + } + + @Test + fun `duplicate paths in a burst collapse to one entry`() = + runTest { + val source = + flow { + emit(m("A.kt")) + delay(20) + emit(m("A.kt")) + delay(20) + emit(m("A.kt")) + } + + val batches = source.coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(1) + assertThat(batches.single().files).containsExactly(f("A.kt")) + } + + @Test + fun `a removal is carried in the batch's removed set`() = + runTest { + val source = + flow { + emit(m("A.kt")) + delay(20) + emit(d("B.kt")) + } + + val batches = source.coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(1) + assertThat(batches.single().files).containsExactly(f("A.kt")) + assertThat(batches.single().removed).containsExactly(f("B.kt")) + } + + @Test + fun `the last event per path wins - create then delete collapses to a removal`() = + runTest { + val source = + flow { + emit(m("A.kt")) + delay(20) + emit(d("A.kt")) + } + + val batches = source.coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(1) + assertThat(batches.single().files).isEmpty() + assertThat(batches.single().removed).containsExactly(f("A.kt")) + } + + @Test + fun `the last event per path wins - delete then recreate collapses to a modification`() = + runTest { + val source = + flow { + emit(d("A.kt")) + delay(20) + emit(m("A.kt")) + } + + val batches = source.coalesceChanges(quietMillis = 150, maxMillis = 1000).toList() + + assertThat(batches).hasSize(1) + assertThat(batches.single().files).containsExactly(f("A.kt")) + assertThat(batches.single().removed).isEmpty() + } + + @Test + fun `a batch is not lost when the consumer is busy at flush time`() = + runTest { + // The quiet-timer flush must not cancel its OWN job before send(): if send() + // then has to suspend (consumer busy), prompt cancellation throws and the batch + // is silently dropped - a stale app. A rendezvous buffer + a busy consumer force + // exactly that suspension. Upstream stays open so the flush comes + // from the timer, not the terminal path. + val source = + flow { + emit(m("A.kt")) + delay(300) // > quiet window: batch 1 flushes via its quiet timer + emit(m("B.kt")) // batch 2's quiet timer fires while the consumer is busy + delay(2000) + } + + val batches = mutableListOf() + source + .coalesceChanges(quietMillis = 150, maxMillis = 1000) + .buffer(Channel.RENDEZVOUS) + .collect { batch -> + batches.add(batch) + delay(1000) // busy well past batch 2's timers: its send() must suspend + } + + assertThat(batches).hasSize(2) + assertThat(batches[0].files).containsExactly(f("A.kt")) + assertThat(batches[1].files).containsExactly(f("B.kt")) + } + + @Test + fun `pending events flush when the upstream completes before the quiet window`() = + runTest { + // Upstream ends immediately after emitting; the terminal flush must still deliver. + val batches = + flowOf(f("A.kt"), f("B.kt")) + .map { WatchEvent.Modified(it) } + .coalesceChanges(quietMillis = 150, maxMillis = 1000) + .toList() + + assertThat(batches.flatMap { it.files }.toSet()).containsExactly(f("A.kt"), f("B.kt")) + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilterTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilterTest.kt new file mode 100644 index 0000000000..b4eaf419f5 --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatchFilterTest.kt @@ -0,0 +1,118 @@ +package org.appdevforall.cotg.quickbuild.domain.watch + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File + +class WatchFilterTest { + @TempDir + lateinit var tempDir: File + + private fun filter(): WatchFilter = + WatchFilter( + watchedRoots = listOf(File(tempDir, "app/src")), + watchedFiles = listOf(File(tempDir, "app/build.gradle.kts")), + ) + + @Test + fun `kt file under the src root is relevant`() { + val file = File(tempDir, "app/src/main/kotlin/Foo.kt") + + assertThat(filter().isRelevant(file)).isTrue() + } + + @Test + fun `a gradle intermediate is not relevant`() { + // Gradle's build/ is a module-root child, so under the production layout (roots are + // /src) it already falls outside every root; it reaches the build-segment test + // only when a caller watches the module dir itself. Both paths must exclude it. + val intermediate = File(tempDir, "app/build/generated/Foo.kt") + + assertThat(filter().isRelevant(intermediate)).isFalse() + assertThat(WatchFilter(watchedRoots = listOf(File(tempDir, "app"))).isRelevant(intermediate)).isFalse() + } + + @Test + fun `a code file in a package named build is relevant`() { + // `build` is a legal Kotlin/Java package name. This filter sits upstream of BOTH the + // inotify and the poll channel, so a wrong drop here means the save reaches nothing at + // all - no build, no batch, no warning, and no poll sweep can rescue it. + val file = File(tempDir, "app/src/main/java/com/example/build/Builders.kt") + + assertThat(filter().isRelevant(file)).isTrue() + assertThat(WatchFilter(watchedRoots = listOf(File(tempDir, "app"))).isRelevant(file)).isTrue() + } + + @Test + fun `file outside all roots is not relevant`() { + val file = File(tempDir, "other/x.kt") + + assertThat(filter().isRelevant(file)).isFalse() + } + + @Test + fun `the watched loose file is relevant`() { + val file = File(tempDir, "app/build.gradle.kts") + + assertThat(filter().isRelevant(file)).isTrue() + } + + @Test + fun `a different loose gradle file not in watchedFiles is not relevant`() { + val file = File(tempDir, "app/settings.gradle.kts") + + assertThat(filter().isRelevant(file)).isFalse() + } + + @Test + fun `temp artifacts under the src root are never relevant`() { + val names = listOf(".hidden.kt", "Main.kt~", "Main.kt.tmp", "x.swp", "y.bak") + + names.forEach { name -> + val file = File(tempDir, "app/src/main/kotlin/$name") + + assertThat(filter().isRelevant(file)).isFalse() + } + } + + @Test + fun `patch and merge droppings under the src root are never relevant`() { + // audit Gap B: a persisted .orig/.rej would otherwise classify UNSUPPORTED and + // force a spurious full Gradle rebaseline instead of the intended quick path. + val names = listOf("Main.kt.orig", "Main.kt.rej") + + names.forEach { name -> + val file = File(tempDir, "app/src/main/kotlin/$name") + + assertThat(filter().isRelevant(file)).isFalse() + } + } + + @Test + fun `JGit checkout dot-prefixed temp files under src are never relevant`() { + // audit rows 9, 10, 12: JGit's DirCacheCheckout writes a `._`-prefixed temp in + // the target dir then renames it onto the target. The dot-prefix drops the temp here, + // so only the final MOVED_TO onto the real path reaches the pipeline. + val names = listOf("._Main.kt", ".merge_file_aBc12", "._strings.xml") + + names.forEach { name -> + val file = File(tempDir, "app/src/main/kotlin/$name") + + assertThat(filter().isRelevant(file)).isFalse() + } + } + + @Test + fun `an unrecognized external-tool temp is relevant here and dropped only downstream`() { + // audit row 14: `sed -i` writes a sibling `sedXXXXXX` temp - no dot-prefix, no known + // suffix, no extension - so the NAME filter deliberately cannot recognize it and it + // passes as relevant. It is dropped later at batch-settle (once it has vanished AND + // has no recognized project-file shape), NOT by widening this name filter. Pinning + // isRelevant==true here guards against a broad name rule that would wrongly also drop + // real files (bug11 covers the downstream drop; see QuickBuildSessionManagerTest). + val sedTemp = File(tempDir, "app/src/main/kotlin/sedAbC123") + + assertThat(filter().isRelevant(sedTemp)).isTrue() + } +} diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconcilerTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconcilerTest.kt new file mode 100644 index 0000000000..c8d195635d --- /dev/null +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/watch/WatcherBatchReconcilerTest.kt @@ -0,0 +1,127 @@ +package org.appdevforall.cotg.quickbuild.domain.watch + +import com.google.common.truth.Truth.assertThat +import org.appdevforall.cotg.quickbuild.domain.ChangedFiles +import org.junit.jupiter.api.Test +import java.io.File + +/** + * Covers the watcher-batch reconciliation decision table directly. The + * QuickBuildSessionManager tests remain the end-to-end regression harness for the same + * behavior. + */ +class WatcherBatchReconcilerTest { + private val source = File("/project/app/src/main/java/com/example/Main.kt") + private val resource = File("/project/app/src/main/res/layout/activity_main.xml") + private val temp = File("/project/app/src/main/java/com/example/sedAbC123") + private val javaResource = File("/project/app/src/main/resources/config.properties") + private val nativeLib = File("/project/app/src/main/jniLibs/arm64-v8a/libnativestub.so") + + private fun reconcile( + batch: ChangedFiles.Known, + existing: Set, + ): ChangedFiles.Known = WatcherBatchReconciler.reconcile(batch) { it in existing } + + @Test + fun `a deleted java resource is kept as a removal, not dropped as noise`() { + // Modifying this file routes to FullGradleBuild because the quick path cannot package + // it; dropping its DELETION left the proxy app serving the deleted content forever. + val result = reconcile(ChangedFiles.Known(files = emptySet(), removed = setOf(javaResource)), existing = emptySet()) + + assertThat(result.removed).containsExactly(javaResource) + } + + @Test + fun `a deleted native library is kept as a removal`() { + val result = reconcile(ChangedFiles.Known(files = emptySet(), removed = setOf(nativeLib)), existing = emptySet()) + + assertThat(result.removed).containsExactly(nativeLib) + } + + @Test + fun `a vanished modified java resource becomes a removal rather than noise`() { + val result = reconcile(ChangedFiles.Known(setOf(javaResource)), existing = emptySet()) + + assertThat(result.files).isEmpty() + assertThat(result.removed).containsExactly(javaResource) + } + + @Test + fun `an extensionless temp is still dropped - the negative control for the packaged-file rule`() { + val result = reconcile(ChangedFiles.Known(files = emptySet(), removed = setOf(temp)), existing = emptySet()) + + assertThat(result.isEmpty).isTrue() + } + + @Test + fun `a modified file that still exists stays modified`() { + val result = reconcile(ChangedFiles.Known(setOf(source)), existing = setOf(source)) + + assertThat(result.files).containsExactly(source) + assertThat(result.removed).isEmpty() + } + + @Test + fun `a vanished modified file with a recognized shape becomes a removal`() { + val result = reconcile(ChangedFiles.Known(setOf(source)), existing = emptySet()) + + assertThat(result.files).isEmpty() + assertThat(result.removed).containsExactly(source) + } + + @Test + fun `a vanished modified file with no recognized shape is dropped as noise`() { + val result = reconcile(ChangedFiles.Known(setOf(temp)), existing = emptySet()) + + assertThat(result.isEmpty).isTrue() + } + + @Test + fun `a watcher-reported removal with a recognized shape is kept`() { + val result = + reconcile( + ChangedFiles.Known(emptySet(), removed = setOf(source)), + existing = emptySet(), + ) + + assertThat(result.files).isEmpty() + assertThat(result.removed).containsExactly(source) + } + + @Test + fun `a watcher-reported removal with no recognized shape is dropped`() { + val result = + reconcile( + ChangedFiles.Known(emptySet(), removed = setOf(temp)), + existing = emptySet(), + ) + + assertThat(result.isEmpty).isTrue() + } + + @Test + fun `a mixed batch reconciles each path independently`() { + val vanishedResource = File("/project/app/src/main/res/values/strings.xml") + val result = + reconcile( + ChangedFiles.Known( + setOf(source, vanishedResource, temp), + removed = setOf(resource), + ), + existing = setOf(source), + ) + + assertThat(result.files).containsExactly(source) + assertThat(result.removed).containsExactly(vanishedResource, resource) + } + + @Test + fun `a persisting file with no recognized shape stays modified for the honest fallback`() { + val javaResource = File("/project/app/src/main/java/com/example/config.properties") + val result = + reconcile(ChangedFiles.Known(setOf(javaResource)), existing = setOf(javaResource)) + + assertThat(result.files).containsExactly(javaResource) + assertThat(result.removed).isEmpty() + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 78cea6d5ca..6623a8c1dc 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -141,6 +141,7 @@ include( ":lsp:ui", ":lsp:xml", ":profiler", + ":quickbuild:core", ":quickbuild:protocol", ":quickbuild:runtime", ":subprojects:aapt2-proto", From bde8b5d554a95bf09717450e2938a7cbd836e4bb Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 21 Aug 2026 22:44:33 -0700 Subject: [PATCH 2/4] =?UTF-8?q?ADFA-4128:=20qb=2005=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20enum=20class/typealias/const=20anchor=20blindness?= =?UTF-8?q?=20(Blocker)=20+=20MOVED=5FTO=20watch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocker (SourceAnnotationScanner TYPE_DECLARATION): the declared-name regex captured "class" for Kotlin `enum class`, and had no branch for `typealias` or top-level `const val`, so edits to such anchor files classified as reload-safe and shipped stale generated code. Fixed the alternation (`enum\s+class` first) and added `typealias` + `const\s+val` branches; added the conservative backstop in AnnotationImpactAnalyzer (a declaration change in a file declaring no recognized name escalates while a processor is active). Covered by AnnotationImpactAnalyzerTest: "adding an entry to an enum class an entity stores escalates", "retargeting a typealias an entity column uses escalates", "bumping a top-level const the database version reads escalates", and "a declaration change in a file declaring no recognized name escalates". Important (AndroidProjectWatcher): directory MOVED_TO never triggered watch registration, leaving a renamed/moved-in package inotify-blind for the session. The recursion gate now fires on CREATE or MOVED_TO; files inside the moved tree are supplied by the poll sweep. FileObserver glue is JVM-inert in this module (isReturnDefaultValues), so the gate carries a precise comment rather than a faked test. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W --- .../quickbuild/data/AndroidProjectWatcher.kt | 7 +- .../domain/annotations/AnnotationImpact.kt | 15 ++- .../annotations/SourceAnnotationScanner.kt | 7 +- .../AnnotationImpactAnalyzerTest.kt | 111 ++++++++++++++++++ 4 files changed, 133 insertions(+), 7 deletions(-) diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt index 0220343aa5..aef40e11ad 100644 --- a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt @@ -145,8 +145,11 @@ class AndroidProjectWatcher( if (path == null) return val changed = File(dir, path) // A new directory (new package, git checkout) needs its own watch, or - // files created inside it later are invisible to inotify. - if (event and CREATE != 0 && changed.isDirectory) { + // files created inside it later are invisible to inotify. MOVED_TO too: a + // renamed/moved-in package arrives as one dir MOVE, and without a watch its + // tree stays inotify-blind for the session. Files already inside the moved + // tree are not re-reported here; the poll sweep picks them up. + if (event and (CREATE or MOVED_TO) != 0 && changed.isDirectory) { registerCreatedTree(changed) } if (event and DELETE_MASK != 0) { diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt index 0972de3070..bc652c0168 100644 --- a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpact.kt @@ -54,10 +54,11 @@ class SwitchableAnnotationImpact( /** * The real [AnnotationImpact]: compares each changed file against the proxy app build's * [AnnotationBaseline], rebaselining when a processor-relevant file changes its annotations or - * declaration surface, is added or deleted, declares an [AnnotationBaseline.anchorNames] type, or - * cannot be scanned. Edits confined to function or initializer bodies stay on the live reload - * path: every processor the profile knows generates from declarations and annotation arguments, - * not statement bodies. + * declaration surface, is added or deleted, declares an [AnnotationBaseline.anchorNames] type, + * changes declarations while declaring no name the scanner recognizes, or cannot be scanned. + * Edits confined to function or initializer bodies stay on the live reload path: every + * processor the profile knows generates from declarations and annotation arguments, not + * statement bodies. * * @param profile which annotations this project's processors consume; an unrecognized processor * widens that to nearly everything. @@ -143,6 +144,12 @@ class AnnotationImpactAnalyzer( if (declaredAnchors.isNotEmpty()) { return "declares ${declaredAnchors.sorted().joinToString()}, read by an annotated declaration" } + // Backstop: a declaration-level change in a file whose declared names the scanner could + // not recognize (top-level `fun`/`val`, or a shape the regex misses) cannot be proven + // outside processor input, so it escalates rather than risk stale generated code. + if (new.declaredTypeNames.isEmpty() && old?.declaredTypeNames.orEmpty().isEmpty()) { + return "declaration change in a file with no recognized declarations" + } return null } diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt index 5cd389c120..292478e0c4 100644 --- a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/annotations/SourceAnnotationScanner.kt @@ -355,8 +355,13 @@ object SourceAnnotationScanner { private val WHITESPACE = Regex("\\s+") private val IMPORT = Regex("^import\\s+(?:static\\s+)?([\\w.]+(?:\\.\\*)?)") private val PACKAGE = Regex("^package\\s+([\\w.]+)") + + // `enum\s+class` must precede `enum`, or Kotlin's `enum class Color` captures the literal + // word "class" as the declared name. `typealias` and top-level `const val` count as + // declarations too: both can be anchors an annotated declaration reads (a `typealias` + // column type, a `const val` database version). private val TYPE_DECLARATION = - Regex("\\b(class|interface|object|enum|record|@interface)\\s+([A-Za-z_][A-Za-z0-9_]*)") + Regex("\\b(enum\\s+class|class|interface|object|enum|record|@interface|typealias|const\\s+val)\\s+([A-Za-z_][A-Za-z0-9_]*)") private val CAPITALIZED = Regex("\\b[A-Z][A-Za-z0-9_]*\\b") /** diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt index 5755130c68..731c12636f 100644 --- a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/domain/annotations/AnnotationImpactAnalyzerTest.kt @@ -284,6 +284,93 @@ class AnnotationImpactAnalyzerTest { assertThat(analyzer.escalation(listOf(fixture.baseEntity))).contains("BaseEntity") } + /** + * Regression: `enum class Color` used to capture the literal word "class" as the declared + * name, so adding an entry to an enum a Room entity stores classified as reload-safe and + * shipped a stale generated converter. + */ + @Test + fun `adding an entry to an enum class an entity stores escalates`() { + val fixture = fixture() + fixture.edit( + fixture.user, + RoomAppFixture.USER.replace("val name: String,", "val name: String,\n\tval color: Color,"), + ) + val color = fixture.write("Color.kt", COLOR) + val baseline = AnnotationBaseline.capture(fixture.all + color, roomProfile) + val analyzer = AnnotationImpactAnalyzer(roomProfile, baseline) + + fixture.edit(color, COLOR.replace("GREEN,", "GREEN,\n\tBLUE,")) + + assertThat(analyzer.escalation(listOf(color))).contains("Color") + } + + /** + * Regression: `typealias` declared no name at all, so retargeting an alias an entity + * column is typed with stayed on live reload while the generated column affinity went + * stale. + */ + @Test + fun `retargeting a typealias an entity column uses escalates`() { + val fixture = fixture() + fixture.edit( + fixture.user, + RoomAppFixture.USER.replace("@PrimaryKey val id: Long,", "@PrimaryKey val id: UserId,"), + ) + val alias = fixture.write("Types.kt", TYPES) + val baseline = AnnotationBaseline.capture(fixture.all + alias, roomProfile) + val analyzer = AnnotationImpactAnalyzer(roomProfile, baseline) + + fixture.edit(alias, TYPES.replace("= String", "= Long")) + + assertThat(analyzer.escalation(listOf(alias))).contains("UserId") + } + + /** + * Regression: a file holding only top-level `const val`s declared nothing, so bumping + * `DB_VERSION` behind `@Database(version = DB_VERSION)` stayed on live reload and the + * installed generated code kept validating the old schema version. + */ + @Test + fun `bumping a top-level const the database version reads escalates`() { + val fixture = fixture() + fixture.edit( + fixture.database, + RoomAppFixture.DATABASE.replace("version = 1", "version = DB_VERSION"), + ) + val constants = fixture.write("Constants.kt", CONSTANTS) + val baseline = AnnotationBaseline.capture(fixture.all + constants, roomProfile) + val analyzer = AnnotationImpactAnalyzer(roomProfile, baseline) + + fixture.edit(constants, CONSTANTS.replace("= 3", "= 4")) + + assertThat(analyzer.escalation(listOf(constants))).contains("DB_VERSION") + } + + /** + * Backstop: a declaration-level change in a file the scanner finds no recognized + * declared name in (here, only a top-level `fun`) cannot be proven outside processor + * input, so it escalates rather than risk stale generated code. + */ + @Test + fun `a declaration change in a file declaring no recognized name escalates`() { + val fixture = fixture() + val analyzer = analyzer(fixture) + val helpers = + fixture.write( + "Helpers.kt", + """ + package com.example.notes + + fun shout(value: String): String { + return value.uppercase() + } + """.trimIndent(), + ) + + assertThat(analyzer.escalation(listOf(helpers))).contains("no recognized declarations") + } + @Test fun `a batch escalates when any one file touches processor input`() { val fixture = fixture() @@ -484,6 +571,30 @@ class AnnotationImpactAnalyzerTest { } private companion object { + val COLOR = + """ + package com.example.notes + + enum class Color { + RED, + GREEN, + } + """.trimIndent() + + val TYPES = + """ + package com.example.notes + + typealias UserId = String + """.trimIndent() + + val CONSTANTS = + """ + package com.example.notes + + const val DB_VERSION = 3 + """.trimIndent() + val HILT_ACTIVITY = """ package com.example.notes From 22f804b040bdb70caa06f3d51b65257953063d64 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Wed, 26 Aug 2026 23:43:45 -0700 Subject: [PATCH 3/4] ADFA-4128 (5/11): address CodeRabbit review - F1717-2 say which port implementations actually live in :app - F1717-4 stop the watcher from re-arming inotify watches after stop() - F1717-5 list TestSourceFilter.kt in the classify README Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FstXxJ5cwWPcvmhZ9vJgJ7 --- quickbuild/core/README.md | 11 ++++-- .../quickbuild/data/AndroidProjectWatcher.kt | 19 +++++++++- .../cotg/quickbuild/domain/classify/README.md | 1 + .../data/AndroidProjectWatcherEdgeTest.kt | 35 +++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/quickbuild/core/README.md b/quickbuild/core/README.md index 5dbb4219ac..7821c05639 100644 --- a/quickbuild/core/README.md +++ b/quickbuild/core/README.md @@ -11,8 +11,13 @@ first. This file only covers what is inside this module. **The domain layer is the Android-free floor.** Nothing under `domain/` imports `android.*` or `androidx.*`, and nothing there takes a `Context`. Every Android capability the module needs is -declared as an interface - a *port* - and implemented in `:app`, wired in one Koin module -([`QuickBuildModule.kt`](../../app/src/main/java/com/itsaky/androidide/di/QuickBuildModule.kt)). +declared as an interface - a *port*. The **Context-bound** implementations live in `:app`, wired in +one Koin module +([`QuickBuildModule.kt`](../../app/src/main/java/com/itsaky/androidide/di/QuickBuildModule.kt)); +`data/` also holds in-module Android adapters that need no `Context`, such as +[`AndroidProjectWatcher`](src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt), +the file-watch port's `FileObserver` implementation. Adding the next file-watch adapter beside it +keeps one concern in one module. **The module as a whole is not Android-free, and is not meant to be.** It is a `com.android.library` with AIDL, and six files under `data/` and `service/` import `android.*` @@ -45,7 +50,7 @@ flowchart TB svcTelemetry["telemetry"] end - subgraph data["data/ - ports (file watch, device paths, daemon); implemented in :app"] + subgraph data["data/ - ports (file watch, device paths, daemon); Context-bound impls in :app, Android adapters here"] direction LR dataPorts["data"] end diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt index aef40e11ad..965cbb2dd1 100644 --- a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt @@ -60,6 +60,16 @@ class AndroidProjectWatcher( private var pipelineJob: Job? = null private var pollJob: Job? = null + /** + * Guarded by [observers]. A directory-CREATE callback already inside [newObserver]'s + * `onEvent` when [stop] runs blocks on that lock and wakes after the clear; without this + * flag it registers a fresh tree of started observers into the now-empty list, which + * [stop] has already finished and can never reach. Their inotify watches then live for the + * process, and enough project close/reopen cycles push registration into the per-uid watch + * limit - after which a save does nothing at all, with no error anywhere. + */ + private var stopped = false + /** * Change fingerprints (path -> lastModified xor size), written by both inotify and the poll * but consulted only by the poll, so a change inotify already delivered is not built twice. @@ -77,6 +87,8 @@ class AndroidProjectWatcher( * since it runs inline on the collecting coroutine. */ override fun start(onBatch: (ChangedFiles.Known) -> Unit) { + // Restart after stop is supported, so clear the latch before anything registers. + synchronized(observers) { stopped = false } pipelineJob = scope.launch { rawEvents @@ -109,6 +121,7 @@ class AndroidProjectWatcher( pipelineJob?.cancel() pipelineJob = null synchronized(observers) { + stopped = true observers.forEach(FileObserver::stopWatching) observers.clear() } @@ -166,10 +179,14 @@ class AndroidProjectWatcher( * Recursive because a tree can arrive whole - a git checkout, an unzip - and watching only its * top level leaves everything deeper on the poll path. * - * @param dir the newly created directory; the walk includes it. + * @param dir the newly created directory; the walk includes it. A no-op once [stop] has + * run, until the next [start]. */ internal fun registerCreatedTree(dir: File) { synchronized(observers) { + // An event that arrived before stop() but got the lock after it must not re-arm + // watches nothing will ever stop. + if (stopped) return // Built fully, then published, so a live iteration of observers never sees a // half-built batch. val fresh = arrayListOf() diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md index df8d8d3653..05eeea9023 100644 --- a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/classify/README.md @@ -5,4 +5,5 @@ Picks the cheapest still-correct build path for a coalesced changed-set, and nam | File | Purpose | | --- | --- | | [`BuildRoute.kt`](BuildRoute.kt) | The route types (`FullGradleBuild`, `ResourcesOnly`, `AssetsOnly`, `CodeOnly`, `CodeAndResources`, `NoOp`, `WarmCompile`), the `recompilesCode` flag, and the `InvalidationReason` enum of why a baseline needs a full Gradle rebuild. | +| [`TestSourceFilter.kt`](TestSourceFilter.kt) | Splits a changed-set into production and test sources (`split` -> `Split`, plus the `isTestSource` path-shape test), so test-only edits do not drive a production build. | | [`ChangeClassifier.kt`](ChangeClassifier.kt) | Routes a changed-set: manifest/Gradle-config/unsupported/non-app-module changes force a full build, otherwise splits code/resource/asset into the cheapest route; also exposes path-shape helpers (`hasRecognizedShape`, `namesResource`). | diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt index 486f931159..4830f387c0 100644 --- a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt @@ -131,6 +131,41 @@ class AndroidProjectWatcherEdgeTest { assertThat(batches).hasSize(1) } + @Test + fun `a create event that lands after stop registers no watches`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val watcher = startWatcher(root, mutableListOf()) + + watcher.stop() + + // The FileObserver thread's CREATE callback blocks on the observers lock during + // stop() and wakes after the clear. Registering here would arm a tree stop() has + // already finished with, so nothing could ever stop those watches again. + File(root, "main/java/com/example").apply { mkdirs() } + watcher.registerCreatedTree(File(root, "main")) + + assertThat(watcher.watchCount()).isEqualTo(0) + } + + @Test + fun `start clears the stopped latch so a reopened project watches again`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val watcher = startWatcher(root, mutableListOf()) + watcher.stop() + + // Same instance, restarted: the latch stop() set has to clear, or a reopened + // project silently never picks up a directory created during the session. + watcher.start {} + val before = watcher.watchCount() + File(root, "main/java").apply { mkdirs() } + watcher.registerCreatedTree(File(root, "main")) + + // main + main/java. + assertThat(watcher.watchCount() - before).isEqualTo(2) + } + private companion object { private const val QUIET_MILLIS = 50L private const val MAX_MILLIS = 1_000L From fbc863b02fb89559cbc0d5a45a552848670cf110 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Mon, 7 Sep 2026 22:08:13 -0700 Subject: [PATCH 4/4] ADFA-4128: make the watcher survive scope cancellation, a throwing handler and a restart Four lifecycle holes in AndroidProjectWatcher, all pinned by tests in AndroidProjectWatcherEdgeTest: - Cancelling the scope left the FileObservers armed; the KDoc claimed otherwise. start() now hooks the scope Job's completion to stop(). https://github.com/appdevforall/CodeOnTheGo/pull/1717#discussion_r3945103296 Fails without the fix: "expected: 0 but was : 1" (watchCount). - A throw from onBatch ended the collecting coroutine while the observers and the poll kept feeding the unlimited channel. Non-cancellation exceptions are logged and collection continues. https://github.com/appdevforall/CodeOnTheGo/pull/1717#discussion_r3945103300 Fails without the fix: "expected: 2 but was : 1" (handler calls). - The observers started before the poll job primed the fingerprints, so a deletion in that window was dropped by reportDeletion and never entered the baseline. The poll job now primes first and then starts the observers, off the session dispatcher. Not pinned by a test: FileObserver is inert on the JVM. https://github.com/appdevforall/CodeOnTheGo/pull/1717#discussion_r3945103306 - stop() closed the raw-event channel for good, so a restarted instance sent every event into a closed channel. The channel is recreated per start(). https://github.com/appdevforall/CodeOnTheGo/pull/1717#discussion_r3945103312 Fails without the fix: "NoSuchElementException: List is empty" (batches.single() after the restart). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K --- .../quickbuild/data/AndroidProjectWatcher.kt | 47 ++++++++--- .../data/AndroidProjectWatcherEdgeTest.kt | 79 ++++++++++++++++++- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt index 965cbb2dd1..4b00ff9836 100644 --- a/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt +++ b/quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcher.kt @@ -1,9 +1,11 @@ package org.appdevforall.cotg.quickbuild.data import android.os.FileObserver +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.DisposableHandle import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay @@ -54,12 +56,16 @@ class AndroidProjectWatcher( private val pollDispatcher: CoroutineDispatcher = Dispatchers.IO, ) : ProjectWatcher { // Unlimited so a burst of inotify events never blocks or drops on a slow drain - coalescing - // downstream collapses the flood into one batch per burst. - private val rawEvents = Channel(Channel.UNLIMITED) + // downstream collapses the flood into one batch per burst. Recreated by every [start]: + // [stop] closes it, and a restarted watcher would otherwise send into a closed channel. + private var rawEvents = Channel(Channel.UNLIMITED) private val observers = mutableListOf() private var pipelineJob: Job? = null private var pollJob: Job? = null + /** Releases the observers when [scope] is cancelled without [stop]; disposed by [stop]. */ + private var scopeCompletion: DisposableHandle? = null + /** * Guarded by [observers]. A directory-CREATE callback already inside [newObserver]'s * `onEvent` when [stop] runs blocks on that lock and wakes after the clear; without this @@ -84,38 +90,51 @@ class AndroidProjectWatcher( * launches the poll sweep. * * @param onBatch invoked once per settled burst on [scope], after restamping; must not block, - * since it runs inline on the collecting coroutine. + * since it runs inline on the collecting coroutine. A throw is logged and the watcher keeps + * collecting; it does not end the session's watch. */ override fun start(onBatch: (ChangedFiles.Known) -> Unit) { // Restart after stop is supported, so clear the latch before anything registers. synchronized(observers) { stopped = false } + val events = Channel(Channel.UNLIMITED) + rawEvents = events + scopeCompletion = scope.coroutineContext[Job]?.invokeOnCompletion { stop() } pipelineJob = scope.launch { - rawEvents + events .consumeAsFlow() .filter { filter.isRelevant(it.file) } .coalesceChanges(quietMillis, maxMillis) .collect { batch -> restampSettled(batch) - onBatch(batch) + try { + onBatch(batch) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // The collector is the only consumer of rawEvents: letting the throw + // end it leaves the observers and the poll feeding an unlimited + // channel nobody drains, for the rest of the session. + log.error("Batch handler failed; the watcher keeps collecting", e) + } } } watchedRoots.filter(File::isDirectory).forEach { root -> root.walkTopDown().filter(File::isDirectory).forEach(::observe) } - // Snapshot before starting: an already-started observer's CREATE handler can - // append to [observers] concurrently, which would throw - // ConcurrentModificationException in a live iteration. - val initial = synchronized(observers) { observers.toList() } - initial.forEach(FileObserver::startWatching) - + // The observers are started by the poll job, after it has primed the fingerprints: + // a deletion delivered before its path is in the map is dropped by reportDeletion and + // then never enters the baseline. The priming is a stat walk, so it stays off the + // caller's thread - start() runs on the session dispatcher, which must not block. pollJob = scope.launch(pollDispatcher) { pollLoop() } log.info("Project watcher started: {} inotify dirs + {}ms poll", observers.size, pollIntervalMillis) } /** Cancels both jobs, stops and drops every observer, and closes the raw-event channel. */ override fun stop() { + scopeCompletion?.dispose() + scopeCompletion = null pollJob?.cancel() pollJob = null pipelineJob?.cancel() @@ -205,6 +224,12 @@ class AndroidProjectWatcher( */ private suspend fun pollLoop() { initFingerprints() // prime without firing: current on-disk state is the baseline + synchronized(observers) { + // Started only now, so every path an observer can report a deletion for is + // already fingerprinted. The latch covers a stop() that got the lock first: its + // clear has already run, and there is nothing left to start. + if (!stopped) observers.forEach(FileObserver::startWatching) + } while (scope.isActive) { delay(pollIntervalMillis) sweep() diff --git a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt index 4830f387c0..1f7078148b 100644 --- a/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt +++ b/quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/data/AndroidProjectWatcherEdgeTest.kt @@ -1,6 +1,9 @@ package org.appdevforall.cotg.quickbuild.data import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy @@ -157,13 +160,87 @@ class AndroidProjectWatcherEdgeTest { // Same instance, restarted: the latch stop() set has to clear, or a reopened // project silently never picks up a directory created during the session. - watcher.start {} + val batches = mutableListOf() + watcher.start(batches::add) + runCurrent() val before = watcher.watchCount() File(root, "main/java").apply { mkdirs() } watcher.registerCreatedTree(File(root, "main")) // main + main/java. assertThat(watcher.watchCount() - before).isEqualTo(2) + + // A change after the restart has to reach the pipeline: stop() closed the raw + // channel, so a restart that kept it would drop every event on the floor. + val source = File(root, "main/java/A.kt").apply { writeText("class A") } + watcher.report(source, fromPoll = false) + settle() + assertThat(batches.single().files).containsExactly(source) + } + + @Test + fun `a batch handler that throws does not end the watch`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val batches = mutableListOf() + val watcher = + AndroidProjectWatcher( + watchedRoots = listOf(root), + watchedFiles = emptyList(), + filter = WatchFilter(listOf(root)), + scope = backgroundScope, + pollIntervalMillis = 3_600_000L, + quietMillis = QUIET_MILLIS, + maxMillis = MAX_MILLIS, + pollDispatcher = StandardTestDispatcher(testScheduler), + ) + var calls = 0 + watcher.start { batch -> + calls++ + if (calls == 1) throw IllegalStateException("handler failed on the first batch") + batches += batch + } + runCurrent() + + val first = File(root, "A.kt").apply { writeText("class A") } + watcher.report(first, fromPoll = false) + settle() + // The collector is the channel's only consumer; a throw that ended it would + // leave every later save feeding a channel nobody drains. + val second = File(root, "B.kt").apply { writeText("class B") } + watcher.report(second, fromPoll = false) + settle() + + assertThat(calls).isEqualTo(2) + assertThat(batches.single().files).containsExactly(second) + } + + @Test + fun `cancelling the scope releases the observers like stop does`() = + runTest { + val root = File(tempDir, "proj").apply { mkdirs() } + val scope = CoroutineScope(Job() + StandardTestDispatcher(testScheduler)) + val watcher = + AndroidProjectWatcher( + watchedRoots = listOf(root), + watchedFiles = emptyList(), + filter = WatchFilter(listOf(root)), + scope = scope, + pollIntervalMillis = 3_600_000L, + quietMillis = QUIET_MILLIS, + maxMillis = MAX_MILLIS, + pollDispatcher = StandardTestDispatcher(testScheduler), + ) + watcher.start {} + runCurrent() + assertThat(watcher.watchCount()).isEqualTo(1) + + // The scope's Job completes once its cancelled children have run their + // cancellation on the dispatcher, which is when the completion handler fires. + scope.cancel() + runCurrent() + + assertThat(watcher.watchCount()).isEqualTo(0) } private companion object {