Skip to content

test: expand frontend API integration coverage - #58

Open
MrDongsls wants to merge 2 commits into
ThinkFlowLab:mainfrom
MrDongsls:test/front-end-tests-enhancement
Open

MrDongsls wants to merge 2 commits into
ThinkFlowLab:mainfrom
MrDongsls:test/front-end-tests-enhancement

Conversation

@MrDongsls

Copy link
Copy Markdown

Propose

Close #46

  • Expand frontend API integration coverage for decision fixtures.
  • Add tests for invalid routes and backend URL prefixes with query parameters.
  • Run the focused frontend test suite in CI and document the command.

Test Plan

System1-Omni Version / Commit: 11f73ea

Test Result

  • cargo fmt --all --check
  • cargo clippy --workspace --locked --all-targets -- -D warnings
  • cargo test -p omni-jev --test frontend --locked — 12 passed
  • cargo test --workspace --locked
  • cargo build --workspace --release --locked

GPU kernel tests were ignored because this environment has no GPU/CUDA runtime.

Self-review

Before marking this PR ready for review or requesting maintainer review, complete
the self-review checklist.
Keep the PR in draft while this work is incomplete.
For agent assistance, use the optional precheck-pr skill.

  • I have reviewed the full diff and addressed the issues I found.
  • I have checked that the change follows the project's architecture and stays focused on the stated purpose.
  • I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
  • I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

Mr.Dong added 2 commits October 1, 2026 17:41
Signed-off-by: Mr.Dong <1042542469@qq.com>
@MrDongsls
MrDongsls marked this pull request as draft October 1, 2026 10:17
@MrDongsls
MrDongsls marked this pull request as ready for review October 1, 2026 10:23

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed and verified locally at 11f73ea.

  • cargo fmt --all --check — clean
  • cargo clippy -p omni-jev --all-targets --locked -- -D warnings — clean
  • cargo test -p omni-jev --test frontend --locked — 12 passed, matching the count reported in the PR description

The three new tests assert real behavior at the transport boundary rather than tautologies: the decision fixtures lock in byte-for-byte preservation in both directions, the invalid-route cases verify the router's 405/404 semantics and that rejected requests never reach the backend (the mock worker records everything through a fallback handler, so that assertion is meaningful), and the prefix + query test covers query-string forwarding under a /worker backend prefix, complementing the existing health-path test. The scope matches #46's proposed first contribution, so closing it on merge is reasonable.

Two non-blocking nits, no changes required:

  1. The new "Frontend API tests" CI step runs tests that cargo test --workspace already runs one step later, so it duplicates coverage for the sake of readable failures. The issue explicitly asked for this, so keeping it is fine — just noting the trade-off.
  2. round_trips_decision_fixtures indexes seen[0] without a seen.len() == 1 check, unlike its sibling tests. Purely stylistic; a failure would still surface.

One housekeeping note: CI currently shows "no checks reported" because this PR touches .github/workflows/ci.yml from a fork — the workflow runs are held until a maintainer approves them in the Actions tab.

This branch has not been deployed

No deployments
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.

[Feature]: Community help wanted — CPU-only API tests in CI

2 participants