Consolidate 29 e2e test binaries into single binary#48
Merged
Conversation
Move all *_e2e_test.rs files into tests/e2e/ directory with a single [[test]] entry in Cargo.toml. This eliminates redundant linking of 29 separate test binaries against the CLI crate, dramatically reducing clean build times. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughIntroduces a consolidated E2E test entry ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kb/iterations/iteration-45-dogfood-fixes.md`:
- Line 5: The document currently sets "status: completed" while the execution
checklist and acceptance items remain unchecked; update the file so the status
matches the checklist in the same commit by either marking "status: in-progress"
(or similar active state) if items remain open, or checking off all remaining
acceptance/checklist items and keeping "status: completed"; ensure you update
the "status: completed" token and the corresponding checklist entries (the
unchecked execution checklist / acceptance items) together in this commit so the
iteration state is unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 399230d8-2184-4e97-b452-e033e5d037f9
📒 Files selected for processing (35)
crates/ff-rdp-cli/Cargo.tomlcrates/ff-rdp-cli/tests/e2e/a11y.rscrates/ff-rdp-cli/tests/e2e/click.rscrates/ff-rdp-cli/tests/e2e/computed.rscrates/ff-rdp-cli/tests/e2e/console.rscrates/ff-rdp-cli/tests/e2e/cookies.rscrates/ff-rdp-cli/tests/e2e/daemon.rscrates/ff-rdp-cli/tests/e2e/daemon_parity.rscrates/ff-rdp-cli/tests/e2e/dom.rscrates/ff-rdp-cli/tests/e2e/dom_tree.rscrates/ff-rdp-cli/tests/e2e/eval.rscrates/ff-rdp-cli/tests/e2e/geometry.rscrates/ff-rdp-cli/tests/e2e/inspect.rscrates/ff-rdp-cli/tests/e2e/launch.rscrates/ff-rdp-cli/tests/e2e/main.rscrates/ff-rdp-cli/tests/e2e/nav_action.rscrates/ff-rdp-cli/tests/e2e/navigate.rscrates/ff-rdp-cli/tests/e2e/network.rscrates/ff-rdp-cli/tests/e2e/page_text.rscrates/ff-rdp-cli/tests/e2e/perf.rscrates/ff-rdp-cli/tests/e2e/perf_compare.rscrates/ff-rdp-cli/tests/e2e/responsive.rscrates/ff-rdp-cli/tests/e2e/screenshot.rscrates/ff-rdp-cli/tests/e2e/scroll.rscrates/ff-rdp-cli/tests/e2e/snapshot.rscrates/ff-rdp-cli/tests/e2e/sources.rscrates/ff-rdp-cli/tests/e2e/storage.rscrates/ff-rdp-cli/tests/e2e/styles.rscrates/ff-rdp-cli/tests/e2e/support/mock_server.rscrates/ff-rdp-cli/tests/e2e/support/mod.rscrates/ff-rdp-cli/tests/e2e/tabs.rscrates/ff-rdp-cli/tests/e2e/type_text.rscrates/ff-rdp-cli/tests/e2e/wait.rskb/iterations/iteration-45-dogfood-fixes.mdkb/iterations/iteration-46-e2e-test-consolidation.md
There was a problem hiding this comment.
Pull request overview
Consolidates the ff-rdp-cli end-to-end test suite from many independent integration test binaries into a single e2e integration test target, reducing repeated linking/compile overhead while keeping existing test behavior intact.
Changes:
- Adds a single
[[test]]target (e2e) pointing attests/e2e/main.rs. - Refactors each e2e test module to share a single
supportmodule viasuper::support. - Updates iteration KB docs to reflect progress/status and task completion.
Reviewed changes
Copilot reviewed 32 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| kb/iterations/iteration-46-e2e-test-consolidation.md | Updates iteration status/tasks/acceptance criteria for the e2e consolidation work. |
| kb/iterations/iteration-45-dogfood-fixes.md | Marks iteration completed and normalizes tags frontmatter into a YAML list. |
| crates/ff-rdp-cli/Cargo.toml | Adds a single [[test]] entry for the consolidated e2e test binary. |
| crates/ff-rdp-cli/tests/e2e/main.rs | New unified e2e test crate root declaring all e2e test modules and shared support. |
| crates/ff-rdp-cli/tests/e2e/support/mod.rs | New shared support module (fixture loading + re-exports). |
| crates/ff-rdp-cli/tests/e2e/support/mock_server.rs | New shared MockRdpServer implementation for e2e tests. |
| crates/ff-rdp-cli/tests/e2e/a11y.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/click.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/computed.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/console.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/cookies.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/daemon.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/daemon_parity.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/dom.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/dom_tree.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/eval.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/geometry.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/inspect.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/launch.rs | Adds a consolidated e2e module containing launch CLI argument smoke tests. |
| crates/ff-rdp-cli/tests/e2e/nav_action.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/navigate.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/network.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/page_text.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/perf.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/perf_compare.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/responsive.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/screenshot.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/scroll.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/snapshot.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/sources.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/storage.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/styles.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/tabs.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/type_text.rs | Updates support imports to use the shared super::support. |
| crates/ff-rdp-cli/tests/e2e/wait.rs | Updates support imports to use the shared super::support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
16
to
+20
| Each `tests/*_e2e_test.rs` file compiles and links as its own binary against the full CLI crate. Clean `cargo test` takes ~71s, with most time spent on redundant linking. The hyalo project had the same problem (31 binaries, 3m13s) and solved it by merging into a single `tests/e2e/mod.rs` — build dropped to ~25s (7.5x speedup). | ||
|
|
||
| ## Tasks | ||
|
|
||
| ### 1. Consolidate e2e test files into single binary [0/5] | ||
| ### 1. Consolidate e2e test files into single binary [5/5] |
There was a problem hiding this comment.
This section says the consolidation pattern is a single tests/e2e/mod.rs, but the tasks (and Cargo.toml) now point to tests/e2e/main.rs. Please update the motivation text to match the actual entrypoint file to avoid confusion for future readers.
- Fix snapshot.rs comment: --depth 2 → --depth 1 to match actual test - Fix daemon_parity.rs: use FF_RDP_HOME instead of HOME/USERPROFILE for consistency with start_daemon() and other tests - Fix mock_server.rs: use labeled break 'conn to exit outer loop on followup write failure instead of only breaking inner for-loop - Remove vacuous #[allow(dead_code)] on mock_server module - Check off iter-45 tasks (iteration already merged) - Update iter-46 status to completed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
*_e2e_test.rsfiles into a singletests/e2e/module with one[[test]]entry in Cargo.tomlTest plan
cargo fmt— no changescargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace -q— all 681 tests pass (295 core + 178 e2e + 197 cli + 9 daemon + 2 live)🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Documentation