Skip to content

Move the inline tests and test-only setters of session.rs (app-logic) into separate files #837

Closed

Description

@smart-ssh-planner

Goal

crates/app-logic/src/session.rs mixes the session type, about 20 test-only setters and a large inline test module. Move the tests and the test-only setters into their own files so the production session code reads on its own. No behaviour changes.

Context

  • crates/app-logic/src/session.rs: 2,678 lines.
  • Inline test module #[cfg(test)] mod tests from :1924 to the end.
  • Test-only setters inside impl Session (:1280-1431), each gated with #[cfg(any(test, feature = "test-support"))] (Spec 0086 A3.3 / ADR 0080 decision 4, Spec 0085 A3.3, Spec 0084 A5): set_*_for_tests, set_sftp_for_tests, … They are reachable from other crates' tests through the test-support feature and must never be callable from production builds (cargo build --workspace builds without test-support and must reject such a call).
  • File naming convention for new test files: <module>/tests.rs or <module>/tests_<topic>.rs; no new *_tests.rs.

Requirements

  • Move mod tests to crates/app-logic/src/session/tests.rs (#[cfg(test)] mod tests;).
  • Move all #[cfg(any(test, feature = "test-support"))] setters of impl Session into a second impl Session block in crates/app-logic/src/session/test_setters.rs, declared from session.rs as #[cfg(any(test, feature = "test-support"))] mod test_setters;. Keep the explanatory comment block (:1280-1288) at the top of that file and keep the per-function #[cfg(any(test, feature = "test-support"))] attributes or replace them by the module-level gate (either is fine; the effective gate must stay exactly any(test, feature = "test-support")).
  • Visibility of each setter stays the same (pub stays pub, pub(crate) stays pub(crate)); callers in other crates compile unchanged.
  • Production methods between the setters (e.g. take_queued_user_messages, request_auto_continue_stop) stay in session.rs.
  • Each new file starts with a //! line.

Acceptance Criteria

  • session.rs contains no inline #[cfg(test)] mod … { block and no *_for_tests method; it has fewer than 2,000 lines.
  • cargo build --workspace (without test-support) still fails to compile a production call to a moved setter: the PR description shows the check (temporarily add a call in a production function, run cargo build -p app-logic, see the error, revert).
  • cargo test -p app-logic --lib -- --list 2>/dev/null | grep -c ': test$' is identical before and after, the set of test names unchanged; the PR description states both numbers.
  • All crates that use the setters (grep -rn '_for_tests' crates/ apps/) compile and their tests pass.
  • No new file matching *_tests.rs.
  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace, cargo build --workspace pass.

Technical Notes

  • set_filter_engine_for_tests, set_ai_provider_for_tests and set_sudo_password_for_tests touch security-relevant session state (see .claude/skills/smart-ssh-coder/references/security-invariants.md). The risk is only in the gate: the moved code must be exactly as unreachable from production as before.
  • Splitting SessionParts into sub-structures is separate work that builds on this issue.

Spec impact: none

Activity

  1. added this to the 0.8.0 milestone on Oct 26, 2026
  2. smart-ssh-coder commented on Oct 26, 2026

    @smart-ssh-coder
    Contributor

    Implementation ready for review. PR #936. Moved session.rs tests to session/tests.rs and test-only setters to session/test_setters.rs (module-level gate unchanged); test count 1175 before/after, production call to a setter still fails to compile; CI green on PR #936.

  3. smart-ssh-reviewer commented on Oct 26, 2026

    @smart-ssh-reviewer

    Review passed – moving on to merge. Pure move verified: 20 setters identical, gate unchanged, 1175 tests before/after, session.rs 1784 lines, CI green.

  4. smart-ssh-coordinator commented on Oct 26, 2026

    @smart-ssh-coordinator
    Contributor

    Merged and branch deleted: PR #936 (squash) in develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    testsTest code and test infrastructure

    Type

    Fields

    Priority

    P2 - Normal

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions