Skip to content

test(tokenless): raise unit test coverage from 75% to 90%#1398

Open
Forrest-ly wants to merge 5 commits into
mainfrom
test/tokenless-coverage-90
Open

test(tokenless): raise unit test coverage from 75% to 90%#1398
Forrest-ly wants to merge 5 commits into
mainfrom
test/tokenless-coverage-90

Conversation

@Forrest-ly

@Forrest-ly Forrest-ly commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • 新增 ~170 个单元测试,将 tokenless workspace 整体代码覆盖率从 75.36% 提升至 90.09%
  • 生产代码仅一处改动:从 run() 提取 run_command() 函数以支持 CLI dispatch 单元测试
  • 未发现生产代码 bug,测试用例主要用于回归保护

各 crate 覆盖率

Crate 覆盖率 说明
tokenless-ccr 94.5% stash 存储核心
tokenless-schema 99.4% 压缩器 + 集成测试
tokenless-stats 95.0% 统计/配置/SLS
tokenless-cli 83.6% CLI + env_check(含 ~165 行不可测代码)

新增测试覆盖

  • tokenless-cli: symlink 信任路径、env var 覆盖(stash/stats DB)、dry-run 模式、Stats Status 分支、record_compression_stats 完整路径、read_input 边界
  • tokenless-schema: stash round-trip(数组/深度/字符串/CJK)、depth truncation、stash 失败回退、builder pattern
  • tokenless-stats: schema migration、corrupt row 容错、SLS Default trait、format_summary/list/compare 边界
  • tokenless-ccr: Default trait、StashError 转换

Test plan

  • cargo test --workspace -- --test-threads=1 全部 443 个测试通过
  • cargo tarpaulin --workspace 覆盖率 90.09%
  • 无生产代码逻辑变更(仅提取函数)

Add ~170 unit tests across all 4 tokenless crates to improve code
coverage from 75.36% to 90.09%, covering edge cases in compression,
stash store, env_check, stats recording, and CLI dispatch.

Key changes:
- Extract run_command() from run() in main.rs for unit testability
- Add symlink, env var override, dry-run mode tests for CLI
- Add stash round-trip, depth truncation, CJK tests for schema compressor
- Add schema migration, corrupt row, SLS writer tests for stats
- Add Default trait, stash error conversion tests for ccr
- Reformat multi-line assertions in integration tests for tarpaulin accuracy

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Forrest-ly Forrest-ly requested a review from shiloong as a code owner July 8, 2026 08:13
@github-actions github-actions Bot added the component:tokenless src/tokenless/ label Jul 8, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shiloong

shiloong commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

PR 评审结论

该 PR 为 4 个 tokenless crate 新增约 170 个单元测试,将覆盖率从 75% 提升至 90%。唯一的生产代码改动是一次干净、最小化的重构:从 run() 中抽取 run_command(command: Commands),使 CLI 分发表可被单元测试覆盖(main.rs:391-396)。query.rssls.rs 的新增内容均在 mod tests 内。重构本身正确,是个好动作。

但若干新增测试在开发机上运行并不安全,另一些在 CI 中会 flaky。合并前应修复。

1. 高危 — 破坏性测试会清空/篡改开发者真实的 tokenless 状态

main_tests.rs 中多个测试调用 run_command(Commands::Stats(...)) / open_recorder() / open_stash_store(None)未隔离 TOKENLESS_STATS_DB / TOKENLESS_STASH_DB,导致 get_db_path() 解析到真实的 ~/.tokenless/stats.db,TokenlessConfig::load() 读取真实配置:

  • `run_command_stats_clear_without_confirm` 以 `yes: true` 在真实 `~/.tokenless/stats.db` 上调用 `recorder.clear()`,会删除运行 `cargo test` 的机器上全部真实统计数据。已在 `main.rs:648-666` 确认:`Clear` 调用 `recorder.clear()`,而该测试未设置任何 DB 覆盖。
  • `run_command_stats_enable_disable` 加载并保存真实 `TokenlessConfig`(`main.rs:706-721`),翻转用户实际的 `stats_enabled` 标志,且测试套件结束后将其留在 enabled/disabled 状态。
  • `run_command_stats_show_existing_record``record_compression_stats_full_path` 向真实 stats DB / SLS 写入记录。
  • `run_command_retrieve_with_marker` / `run_command_retrieve_bare_hash` 调用 `open_stash_store(None).stash(...)`,污染真实 `~/.tokenless/stash.db`。

这正是 env-override 测试已经正确采用的模式(如 `get_db_path_with_env_override` 将 `TOKENLESS_STATS_DB` 指向 tempdir)。stats/stash 命令测试应照此办理——在测试中将该 env var 指向一个 tempdir DB,结束时移除。当前情况下,在开发机上 `cargo test --workspace` 是个隐患。

2. 中危 — 依赖网络的单元测试

  • `check_network_https_resolves` 实时访问 `https://httpbin.org/status/200\`。
  • `write_test_spec` 的 `MissingTool` 条目含 `"network": ["https://httpbin.org/status/418"]`,而多个 `run_*` 测试传入 `all=true` 或检查 `MissingTool`,因此会发起真实出站 HTTP 调用。

这些在离线/沙箱 CI 中会 flaky,且并非真正的单元测试。建议用 `#[ignore]` 配合 network feature 门控,或 mock `check_network`。

3. 中危 — env var 变更无序列化保证,存在竞态

workspace 为 edition 2024,`std::env::set_var`/`remove_var` 为 `unsafe`——测试中已正确包裹。但正确性完全依赖于测试计划中的 `--test-threads=1`。直接 `cargo test`(默认并行)会让这些进程级 env 变更产生竞态:`TOKENLESS_STATS_DB`、`TOKENLESS_STASH_DB`、`TOKENLESS_STATS_ENABLED`、`TOKENLESS_SLS_ENABLED`、`TOKENLESS_COMPRESSION_ENABLED`、`TOKENLESS_TOOL_READY_SPEC` 均在多测试间被变更,且无 `serial_test`/mutex。建议要么引入 `serial_test`(或 `std::sync::Mutex` 守卫),要么将被测函数重构为接受参数而非读取进程 env;至少在测试旁注释单线程要求。

4. 低危 — 100 MB 文件写入

`read_input_file_too_large` 向 tempdir 写入 100 MiB + 1。作为单元测试过慢过占磁盘;建议加 `#[ignore]`,或用更小的边界值同样触发 limit 检查。

结论

测试覆盖的新增价值很高,生产代码重构也干净。但发现 #1 为阻塞项——在贡献者机器上运行该测试套件会静默清空其真实 `~/.tokenless/stats.db` 并翻转其配置。合并前请修复 stats/stash 命令测试的 DB 隔离;#2/#3 可同步处理或作为紧随跟进项。

Forrest-ly and others added 2 commits July 8, 2026 16:34
Tests that mutated TOKENLESS_SLS_ENABLED via set_var raced with the
Stats Enable/Disable test (which calls TokenlessConfig::load() then
save()), writing sls_enabled=false to ~/.tokenless/config.json. The
tokenless-stats test binary reads that file concurrently and fails.

Replaced 9 unsafe env-var-mutating tests with a safe alternative that
calls Stats Disable then Status to cover the DISABLED display branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add TempDbGuard to redirect stats/stash DB to temp dirs under $HOME
- Serialize env var mutations with Mutex in both main_tests and env_check_tests
- Remove tests that write to real config (save() path cannot be redirected)
- Mark network-dependent tests with #[ignore]
- Remove httpbin URL from write_test_spec to prevent real HTTP calls
- Reduce read_input_file_too_large from 100MB to 64MB+1 (matches MAX_INPUT_BYTES)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Forrest-ly

Copy link
Copy Markdown
Collaborator Author

@shiloong 感谢详细的 review,所有 4 个问题已在最新提交 (4c904ce7) 中修复:

1. ✅ 高危 — DB 隔离 (已修复)

新增 TempDbGuard 结构体,创建时:

  • 获取全局 Mutex 锁,确保 env var 修改不会并发竞争
  • $HOME/.tokenless-test-{nanos}/ 下创建临时目录(路径在 home 下,能通过 validate_db_path 校验)
  • 设置 TOKENLESS_STATS_DBTOKENLESS_STASH_DB 指向临时目录
  • Drop 时自动清理 env var 和临时目录

所有写 DB 的测试已改用 TempDbGuard,不再触碰真实的 ~/.tokenless/stats.dbstash.db

另外,run_command_stats_enable_disablerun_command_stats_status_after_disable 两个测试已删除——因为 TokenlessConfig::save() 写入 config_path() 硬编码路径,无法通过 env var 重定向,这两个测试必然修改真实配置文件。

2. ✅ 中危 — 网络依赖 (已修复)

  • check_network_https_outboundcheck_network_https_resolves 已添加 #[ignore]
  • write_test_specMissingToolnetwork 字段从 ["https://httpbin.org/status/418"] 改为 [],避免 run_* 测试触发真实 HTTP 请求

3. ✅ 中危 — env var 竞态 (已修复)

  • main_tests.rs: 所有 env var 操作均通过 TempDbGuardENV_MUTEX 串行化;纯路径计算测试也加了 ENV_MUTEX.lock() 防止与 guard 竞争
  • env_check_tests.rs: 新增独立的 ENV_MUTEX,所有 19 个使用 set_var("TOKENLESS_TOOL_READY_SPEC"/TOKENLESS_PACKAGE_MANAGER") 的测试均已加锁

4. ✅ 低危 — 100 MB 文件 (已修复)

read_input_file_too_large100 * 1024 * 1024 + 1 改为 64 * 1024 * 1024 + 1,恰好比 MAX_INPUT_BYTES (64 MiB) 大 1 字节,触发同样的限制检查且更快更省磁盘。


测试结果:432 passed, 0 failed, 2 ignored (网络测试)。

validate_db_path canonicalizes the parent directory, which fails on CI
where ~/.tokenless/ does not exist yet. Create the directory first.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:tokenless src/tokenless/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants