1jehuang / 1jehuang/jcode

TUI lib tests share process-global state, so they only pass serially

Open
#596 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

priority: medium tech-debt triage: reproducible
Dominant language
Rust
Stars
19.9k
Forks
2.3k
Avg merge
2d 7h
Merged PRs (30d)
30

Description

Follow-up to #592, which enabled the TUI library tests in CI. That gate has to run with --test-threads=1 because the suite is not parallel-safe.

Evidence

  • cargo test -p jcode-tui --lib -- --test-threads=1 → 1977 passed, 0 failed.
  • Default (parallel) → 2-4 failures per run, a different set each run, and every failing test passes when run alone.

Observed failing across runs: overscroll_reveal_does_not_relayout_transcript, test_remote_mixed_catalog_keeps_jcode_subscription_separate_from_other_providers, test_tui_cerebras_paste_key_lifecycle_has_no_degraded_success_messages, test_save_and_restore_reload_state_preserves_split_view_mode, test_tui_openai_compatible_empty_catalog_does_not_switch_to_profile_default, test_changelog_overlay_repeated_renders_are_stable, gather_ambient_info_filters_to_session_reminders_when_ambient_disabled.

Contended state

  • the process-global model catalog / route memo
  • AMBIENT_INFO_CACHE, whose stale-while-revalidate path spawns a background thread that reloads under whatever JCODE_HOME is current when it runs, not when it was queued
  • render state (partly addressed already by the shared render_state_test_lock)
  • JCODE_HOME itself, when a test sets it without holding the shared env lock

helpers_tests.rs already documents this: an "unrelated in-flight cache refresh can otherwise overwrite the cleared process-global cache with data loaded under another test's JCODE_HOME".

What worked in jcode-base

Two tests with exactly this shape were fixed by scoping, not redesign:

  • wrap the body in with_clean_provider_test_env
  • hold the shared test-env lock plus a private JCODE_HOME (matching the neighbouring test that already did)

cargo test -p jcode-base went from 1-2 intermittent failures per run to 1130 passing across three consecutive runs.

Why it is worth doing

  1. Ordering-dependent tests hide real regressions: a genuine break looks like the usual flake.
  2. Serial costs ~40s in CI where parallel would be ~15s.
  3. The --skip list in the CI step is blunt. Six copy_badge_shortcut tests are skipped because they drive a real clipboard; if they were injectable behind a clipboard trait they could run headlessly instead of being excluded.

Suggested order

  1. Scope the catalog/ambient tests the way the jcode-base ones were.
  2. Make the ambient cache refresh capture its JCODE_HOME at queue time, or skip the background path entirely under cfg(test).
  3. Abstract the clipboard so the copy-badge tests can drop out of the CI skip list.
  4. Only then drop --test-threads=1.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with helpers_tests.rs and reproduce the issue using the serial and default cargo test commands for jcode-tui. Trace the listed catalog, ambient-cache, render-state, and JCODE_HOME tests, along with the jcode-base scoping approach. Done means the library tests pass reliably in parallel so CI no longer needs --test-threads=1.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.