Skip to content

[codex] Run all tests on pull requests#44

Draft
ZeroPointSix wants to merge 3 commits into
masterfrom
codex/run-all-tests-on-pr
Draft

[codex] Run all tests on pull requests#44
ZeroPointSix wants to merge 3 commits into
masterfrom
codex/run-all-tests-on-pr

Conversation

@ZeroPointSix

Copy link
Copy Markdown
Owner

Summary

Make the PR CI path run all test suites that currently exist in the repository, not only the frontend Node tests.

Changes

  • Added npm run test as the canonical full test command.
  • Added npm run test:rust to execute Rust tests through cargo test --manifest-path src-tauri/Cargo.toml.
  • Updated the CI check-and-test job to install Rust and required Tauri Linux dependencies before running tests.
  • Changed the CI test step from npm run test:unit to npm run test, so PRs run both Node unit tests and Rust tests.

Why

The repository already has Rust #[test] coverage in backend modules, but the PR CI path only ran node --test. This PR aligns CI with the intended policy: every PR should run the full available test suite before build jobs continue.

Validation

  • CI workflow remains triggered on pull_request to main/master.
  • Expected CI path now runs npm run check, npm run test (Node + Rust), frontend build, then Tauri build matrix.
  • Local checkout/test execution was not available in this container because direct GitHub clone/download requests are blocked, so the PR CI should be used as the authoritative run.

Copy link
Copy Markdown
Owner Author

总体结论:这个 PR 的方向正确,能让 PR CI 覆盖现有 Rust 测试;未发现会改变运行时行为的阻塞问题,但有一个格式细节建议在合并前修掉。

关键发现:

  • 中:prompt-launcher/package.json 现在缺少文件末尾换行(diff 显示 No newline at end of file)。这类问题通常不会影响 npm run test,但容易让格式检查、编辑器配置或后续 diff 保持噪声。建议补回末尾换行,并在 CI 中确认格式/检查命令不会因此失败。
  • 低:CI 现在会在 check-and-test job 中安装 Rust、Tauri Linux 依赖并运行 cargo test,这与 PR 目标一致。需要注意的是这会增加 PR CI 耗时;如果后续变慢明显,可以再考虑缓存 apt 或拆分 Rust test job,但不建议为了速度牺牲当前覆盖。
  • 低:npm run test 串行执行 test:unittest:rust,语义清楚,也方便本地复现完整测试路径。

优先级建议:

  • 阻塞:无。
  • 中:补回 package.json 末尾换行。
  • 低:合并前以 GitHub Actions 的实际 PR run 作为权威验证,确认新增 Tauri 依赖包在 runner 镜像上可安装。

后续建议:

  • 如果这条 CI 后续承担合并门禁,建议在 README 或贡献说明里记录 npm run test 现在代表 Node + Rust 的完整测试集合。

@ZeroPointSix

Copy link
Copy Markdown
Owner Author

CodeXWeb 本轮自动巡检已认领,正在处理本 PR 的 review 修改意见。

本轮只会处理这一项:#44。处理范围限定为 review comment 4670502302 提到的 prompt-launcher/package.json 文件末尾换行问题;完成后会在本 PR 回写改动、验证结果和风险,不会自动合并。

@ZeroPointSix

ZeroPointSix commented Jun 10, 2026

Copy link
Copy Markdown
Owner Author

CodeXWeb follow-up for this scheduled patrol round:

Validation completed:

Rust/Tauri validation note: I installed the Linux dependencies and stable Rust 1.96.0 in the remote sandbox. Full local cargo test still could not complete because rustc was killed by SIGKILL while compiling glib, even with --jobs 1 and reduced debug info. The pushed commit's GitHub Actions CI did complete successfully.

No auto-merge performed.

@ZeroPointSix ZeroPointSix left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

总体结论:本轮巡检已覆盖 PR #44 最新提交 3da6b10 的 CI 与 package.json 改动;当前未发现阻塞问题,新增测试入口与 CI 执行路径整体一致。

关键发现:

  • 完整性:prompt-launcher/package.json 新增 test 作为聚合入口,并串行执行 test:unittest:rust.github/workflows/ci.ymlcheck-and-test job 已从 npm run test:unit 改为 npm run test,能把现有 Rust 测试纳入 PR CI,符合 PR 描述目标。
  • 环境匹配:CI 在运行 npm run test 前安装 Rust stable、Rust cache 和 Ubuntu/Tauri 依赖,避免 cargo test --manifest-path src-tauri/Cargo.toml 因缺少 Rust 或常见系统包直接失败。工作目录仍限定在 ./prompt-launcher,与 package scripts 和 manifest 相匹配。
  • 旧评论已处理:此前普通评论中指出的 prompt-launcher/package.json 文件末尾换行问题,在最新文件内容中已经恢复,不再构成待处理项。
  • 验证状态:查询到最新提交 3da6b1061883ab2222d3baf338964e82ea84ee58 对应的 PR workflow run CI #58 已完成且结论为 success。

优先级建议:

  • 阻塞:无。
  • 高:无。
  • 中:无。
  • 低:如果后续 CI 耗时明显上升,可以再考虑把 Rust 测试拆成独立 job 或进一步缓存,但不建议在本 PR 中削弱测试覆盖。

后续建议:

  • 合并前保留 GitHub Actions 作为权威验证来源;如果这个 npm run test 语义会被贡献者本地使用,建议后续在贡献说明或 README 中记录它现在代表 Node + Rust 的完整测试集合。

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.

2 participants