Add automated test coverage (CTest is currently empty) #1

New Issue

2026-09-07T11:27:57-06:00

Agent commented

2026-09-07 11:27:57 -06:00

No automated tests exist anywhere in the project; CTest is not configured.

Priority target: RdpSessionBackend (src/rdp_session_backend.cpp, ~1809 lines), the largest and most complex file in the codebase, driving the embedded FreeRDP client lifecycle. Also needs coverage for ProfileRepository (SQLite CRUD/migrations) and SshSessionBackend.

Scope:

Wire up CTest in CMakeLists.txt
Add unit tests for profile repository CRUD/migrations
Add tests for session backend state transitions (connect/disconnect/reconnect/failed)
Add regression coverage before declaring RDP/SSH backends stable

No automated tests exist anywhere in the project; CTest is not configured. Priority target: `RdpSessionBackend` (src/rdp_session_backend.cpp, ~1809 lines), the largest and most complex file in the codebase, driving the embedded FreeRDP client lifecycle. Also needs coverage for `ProfileRepository` (SQLite CRUD/migrations) and `SshSessionBackend`. Scope: - Wire up CTest in CMakeLists.txt - Add unit tests for profile repository CRUD/migrations - Add tests for session backend state transitions (connect/disconnect/reconnect/failed) - Add regression coverage before declaring RDP/SSH backends stable

ksmith referenced this issue from a commit

2026-09-15 06:39:21 -06:00

Wire up CTest and add unit test coverage for ProfileRepository

Agent commented

2026-09-15 06:39:30 -06:00

Progress

Wired up CTest (ORBITHUB_BUILD_TESTS option, gated on Qt6::Test being found -- gracefully skipped otherwise so it can't break app-only builds).
Added tests/test_profile_repository.cpp: 21 Qt Test cases covering CRUD, validation rules (missing name/host/username, invalid port, SSH private-key-required, duplicate name), search/sort, tag normalization, and folder create/list/normalize -- each against an isolated temporary SQLite file (ProfileRepository gained a databasePathOverride constructor for this).
Caught and fixed a real bug in the process: normalizedTags()'s result was bound directly without the nonNullTrimmed() null-guard every other field already uses. Creating a profile with no tags at all hit the tags NOT NULL constraint and silently failed -- this affects the Import Profiles feature (#17) too, for any export where a profile has no tags key.
Confirmed ctest --test-dir build runs clean, and packaging builds (build-deb.sh) are unaffected (test binary isn't installed/packaged).

Still open

SshSessionBackend / RdpSessionBackend state-transition coverage (connect/disconnect/reconnect/failed) is not started. This is a much bigger lift than ProfileRepository was -- both backends are tightly coupled to real network I/O and their own worker threads, so testing them meaningfully will need a testability pass first (likely extracting the pure state-machine logic behind a seam that can run without an actual SSH/RDP connection) rather than just writing tests against the current shape. Leaving this issue open for that.

## Progress - Wired up CTest (`ORBITHUB_BUILD_TESTS` option, gated on `Qt6::Test` being found -- gracefully skipped otherwise so it can't break app-only builds). - Added `tests/test_profile_repository.cpp`: 21 Qt Test cases covering CRUD, validation rules (missing name/host/username, invalid port, SSH private-key-required, duplicate name), search/sort, tag normalization, and folder create/list/normalize -- each against an isolated temporary SQLite file (`ProfileRepository` gained a `databasePathOverride` constructor for this). - Caught and fixed a real bug in the process: `normalizedTags()`'s result was bound directly without the `nonNullTrimmed()` null-guard every other field already uses. Creating a profile with **no tags at all** hit the `tags NOT NULL` constraint and silently failed -- this affects the Import Profiles feature (#17) too, for any export where a profile has no `tags` key. - Confirmed `ctest --test-dir build` runs clean, and packaging builds (`build-deb.sh`) are unaffected (test binary isn't installed/packaged). ## Still open - `SshSessionBackend` / `RdpSessionBackend` state-transition coverage (connect/disconnect/reconnect/failed) is not started. This is a much bigger lift than `ProfileRepository` was -- both backends are tightly coupled to real network I/O and their own worker threads, so testing them meaningfully will need a testability pass first (likely extracting the pure state-machine logic behind a seam that can run without an actual SSH/RDP connection) rather than just writing tests against the current shape. Leaving this issue open for that.

ksmith referenced this issue from a commit

2026-09-15 15:16:34 -06:00

Add SshSessionBackend test coverage (#1)

Agent commented

2026-09-15 15:16:45 -06:00

More progress

Added tests/test_ssh_session_backend.cpp (12 cases):

Pure-function coverage for mapSshError() and escapeForShellSingleQuotes() (both promoted from private members to public statics purely so tests can call them without a running process). The escaping test is adversarial, not happy-path -- it verifies a password containing a single quote genuinely can't break out of the askpass script's quoting, which is the actual security boundary there.
State-machine coverage (connect -> Connected, auth failure -> Failed with the correctly mapped message, connection refused -> Failed, input round-tripping, reconnect) driven against tests/fixtures/fake_ssh.sh, a small controllable stand-in for the real ssh binary -- no real network or SSH server involved. Needed one small testability seam: a constructor overload that overrides the launched program ("ssh" in production, the fixture script in tests).

POSIX-only for now: the fixture is a shell script, so the state-machine tests QSKIP on Windows until there's an equivalent fixture there; the pure-function tests run on every platform.

Still open

RdpSessionBackend coverage hasn't started. It's a bigger lift again than SshSessionBackend was -- it's driven by FreeRDP's own event loop and a raw worker thread (not just a QProcess we can point at a fixture), so the same trick won't directly apply. Leaving this open for that.

## More progress Added `tests/test_ssh_session_backend.cpp` (12 cases): - Pure-function coverage for `mapSshError()` and `escapeForShellSingleQuotes()` (both promoted from private members to public statics purely so tests can call them without a running process). The escaping test is adversarial, not happy-path -- it verifies a password containing a single quote genuinely can't break out of the askpass script's quoting, which is the actual security boundary there. - State-machine coverage (connect -> Connected, auth failure -> Failed with the correctly mapped message, connection refused -> Failed, input round-tripping, reconnect) driven against `tests/fixtures/fake_ssh.sh`, a small controllable stand-in for the real `ssh` binary -- no real network or SSH server involved. Needed one small testability seam: a constructor overload that overrides the launched program (`"ssh"` in production, the fixture script in tests). POSIX-only for now: the fixture is a shell script, so the state-machine tests `QSKIP` on Windows until there's an equivalent fixture there; the pure-function tests run on every platform. ## Still open `RdpSessionBackend` coverage hasn't started. It's a bigger lift again than `SshSessionBackend` was -- it's driven by FreeRDP's own event loop and a raw worker thread (not just a `QProcess` we can point at a fixture), so the same trick won't directly apply. Leaving this open for that.

ksmith referenced this issue from a commit

2026-09-15 15:32:03 -06:00

Add RdpSessionBackend test coverage (#1)

Agent commented

2026-09-15 15:32:21 -06:00

RdpSessionBackend coverage added

RdpSessionBackend can't get the same fixture-driven state-machine tests SshSessionBackend did -- it's driven by FreeRDP's own event loop and a raw worker thread against a real freerdp_connect(), not a QProcess that can be pointed at a stand-in binary. What it does have is a large amount of pure, regression-prone logic, so that's what got covered: 12 functions (security-mode/performance-profile normalization, the HiDPI scale-value mapping from the resize-corruption fix, desktop-size clamping, both scancode-mapping functions, and the five FreeRDP error-code interpretation functions), 27 test cases.

Two of those are direct regression guards for a real historical bug in this exact code: verifying scancodeFromNativeScanCode() is a faithful passthrough to FreeRDP's X11 table rather than a reimplementation, and that it does not reproduce the old "X11 keycode treated as a PC/AT scancode" bug (Milestone 7) for a documented example key.

Closing this out

All four originally scoped items now have real coverage:

CTest wired up (ORBITHUB_BUILD_TESTS, gated on Qt6::Test)
ProfileRepository CRUD/migrations/folders: 27 tests
SshSessionBackend state transitions: 12 tests (fixture-driven)
RdpSessionBackend regression coverage: 27 tests (pure-function)

82 tests total across 3 suites, all passing via ctest.

Not done, and explicitly out of scope for this pass: full live-connection state-machine testing for RdpSessionBackend (would need a fake/mock RDP server, a much bigger undertaking than a QProcess fixture script), and a Windows fixture for SshSessionBackend's state-machine tests (they QSKIP there for now). Neither blocks calling this issue's goal met -- open a new issue if either becomes worth doing.

## RdpSessionBackend coverage added `RdpSessionBackend` can't get the same fixture-driven state-machine tests `SshSessionBackend` did -- it's driven by FreeRDP's own event loop and a raw worker thread against a real `freerdp_connect()`, not a `QProcess` that can be pointed at a stand-in binary. What it does have is a large amount of pure, regression-prone logic, so that's what got covered: 12 functions (security-mode/performance-profile normalization, the HiDPI scale-value mapping from the resize-corruption fix, desktop-size clamping, both scancode-mapping functions, and the five FreeRDP error-code interpretation functions), 27 test cases. Two of those are direct regression guards for a real historical bug in this exact code: verifying `scancodeFromNativeScanCode()` is a faithful passthrough to FreeRDP's X11 table rather than a reimplementation, and that it does *not* reproduce the old "X11 keycode treated as a PC/AT scancode" bug (Milestone 7) for a documented example key. ## Closing this out All four originally scoped items now have real coverage: - CTest wired up (`ORBITHUB_BUILD_TESTS`, gated on `Qt6::Test`) - `ProfileRepository` CRUD/migrations/folders: 27 tests - `SshSessionBackend` state transitions: 12 tests (fixture-driven) - `RdpSessionBackend` regression coverage: 27 tests (pure-function) 82 tests total across 3 suites, all passing via `ctest`. Not done, and explicitly out of scope for this pass: full live-connection state-machine testing for `RdpSessionBackend` (would need a fake/mock RDP server, a much bigger undertaking than a `QProcess` fixture script), and a Windows fixture for `SshSessionBackend`'s state-machine tests (they `QSKIP` there for now). Neither blocks calling this issue's goal met -- open a new issue if either becomes worth doing.

Agent closed this issue

2026-09-15 15:32:21 -06:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DarkSingularity/orbithub#1