Skip to content

fix(bazel): grant common_test_fixtures friend access to jvm_main_lib - #272

Merged
tstapler merged 3 commits into
mainfrom
fix/bazel-jvm-tests-toml-associates
Aug 14, 2026
Merged

fix(bazel): grant common_test_fixtures friend access to jvm_main_lib#272
tstapler merged 3 commits into
mainfrom
fix/bazel-jvm-tests-toml-associates

Conversation

@tstapler

Copy link
Copy Markdown
Owner

Summary

  • common_test_fixtures (kmp/src/commonTest/kotlin/BUILD.bazel) compiles SectionManifestParserTest.kt, which references the internal TOML_PARSING_SUPPORTED expect/actual declaration in jvm_main_lib.
  • Bazel's rules_kotlin only exposes internal declarations across a friend-module boundary (associates), not plain deps — this target had deps only, causing //kmp/src/jvmTest/kotlin:jvm_tests to fail to compile and the "Bazel JVM tests" CI job to be red on main.
  • Adds associates = ["//kmp/src/jvmMain/kotlin:jvm_main_lib"], mirroring the existing pattern on jvm_tests and android_unit_test_lib.

Test plan

  • CI "Bazel JVM tests" job passes on this PR

SectionManifestParserTest references the internal TOML_PARSING_SUPPORTED
expect/actual declaration. Bazel's kt_jvm_library only exposes internal
declarations across a friend-module (associates) boundary, not plain deps —
common_test_fixtures had deps only, so //kmp/src/jvmTest/kotlin:jvm_tests
failed to compile. Mirrors the same associates attribute already used by
jvm_tests and android_unit_test_lib.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tstapler
tstapler force-pushed the fix/bazel-jvm-tests-toml-associates branch from 0d7e20c to e9204f7 Compare August 14, 2026 16:51
@tstapler
tstapler marked this pull request as ready for review August 14, 2026 16:51
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

JVM Load Benchmark (Desktop)

Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Comparing 13615dd6 (this PR) vs 79724e1f (baseline)
Graph config: xlarge — 230 pages

Metric This PR Baseline Delta
Phase 1 TTI ↓ 1ms 0ms +1ms ⚠️
Phase 2 background ↓ 0ms 0ms 0 (0%)
Phase 3 index ↓ 1ms 1ms 0 (0%)
Total ↓ 2ms 1ms +1ms (+100%) ⚠️
Write p95 (baseline) ↓ 24ms 17ms +7ms (+41%) ⚠️
Write p95 (under load) ↓ n/a 1ms
Jank factor ↓ n/a 0.06x
↓ lower is better
Flamegraphs (this PR) **Allocation** — object allocation pressure (JDBC/SQLite churn)

Alloc flamegraph not available

CPU — method-level hotspots by on-CPU time

CPU flamegraph not available

Top allocation hotspots (this PR) `36.2%` byte[]_[k] `8.1%` java.lang.String_[k] `6.6%` int[]_[k] `6.1%` java.util.LinkedHashMap$Entry_[k] `4.3%` java.lang.Object[]_[k]
Top CPU hotspots (this PR) `96.9%` /usr/lib/x86_64-linux-gnu/libc.so.6 `1.2%` /tmp/sqlite-3.51.3.0-f8e15520-d00a-407c-a0e1-07fb5827bf27-libsqlitejdbc.so `0.5%` __libc_pwrite `0.2%` fsync `0.2%` pthread_cond_signal

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Android Load Benchmark

Instrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph.

Comparing 13615dd6 (this PR) vs 79724e1f (baseline)
Device: API 30 x86_64 emulator — 530 pages loaded

Graph Load

Metric This PR Baseline Delta
Phase 1 TTI ↓ 40ms 32ms +8ms (+25%) ⚠️
Phase 3 index ↓ 3775ms 3936ms -161ms (-4%) ✅

Interactive Write Latency (during Phase 3)

Metric This PR Baseline Delta
Write p95 (baseline) ↓ 12ms 11ms +1ms (+9%) ⚠️
Write p95 (during phase 3) ↓ 9ms 23ms -14ms (-61%) ✅
Jank factor ↓ 0.75x 2.09x -1.34x (-64%) ✅
Concurrent writes ↑ 18 19 -1ms (-5%) ⚠️

SAF I/O Overhead (ContentProvider vs direct File read)

Measures Binder IPC cost added by ContentResolver per readFile() call.
Real SAF via ExternalStorageProvider will be higher on device; this is a lower bound.

Metric This PR Baseline Delta
Direct read / file ↓ 0.0ms 0.0ms 0 (0%)
Provider read / file ↓ 0.3ms 0.2ms +0ms (+65%) ⚠️
IPC overhead ratio ↓ 10x 6x +4x (+67%) ⚠️
↓ lower is better · ↑ higher is better

tstapler and others added 2 commits August 14, 2026 10:09
rules_kotlin's kt_jvm_library rejects a target listed in both
associates and deps simultaneously (fail_if_invalid_associate_deps),
which broke the prior commit's fix — associates already grants
friend access transitively, so deps must not duplicate it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
renamePage_clears_the_pendingConflicts_entry_for_the_old_file_path used a
real PlatformFileSystem rooted at a temp directory, so GraphWriter.renamePage
performed genuine disk reads/writes/deletes under a Mutex inside the test's
withTimeout(2_000) budget. Under CI resource contention that real I/O
occasionally exceeded 2s, causing an intermittent
TimeoutCancellationException (CI run 31822728648, job 94839667806) unrelated
to the associates/deps fix in this PR.

The test only asserts on ViewModel state (pendingConflicts), not actual file
contents, so swapping in the fixture's in-memory FakeFileSystem removes the
wall-clock-dependent I/O while still exercising the same
GraphWriter/BacklinkRenamer/ViewModel code paths deterministically.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tstapler
tstapler merged commit 671eccd into main Aug 14, 2026
18 checks passed
@tstapler
tstapler deleted the fix/bazel-jvm-tests-toml-associates branch August 14, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant