Skip to content

Commit de41a57

Browse files
josecelanojosecelano
committed
docs(issues): add implementation specs for EPIC #1525 overhaul persistence
Add the EPIC planning doc and eight sub-issue specs covering the full persistence overhaul: - #1525: EPIC – Overhaul Persistence (delivery strategy + ordering) - #1525-01: Persistence test coverage (DB compatibility matrix) - #1525-02: qBittorrent E2E test runner - #1525-03: Persistence benchmarking framework - #1525-04: Split persistence traits (narrow interfaces) - #1525-05: Migrate SQLite and MySQL drivers to sqlx - #1525-07: Align Rust and DB types (u32→u64, INTEGER→BIGINT) - #1525-08: Add PostgreSQL driver Also adds new technical terms to project-words.txt (dbname, isready, VARCHAR) required for the cspell spell-checker to pass.
1 parent a95af24 commit de41a57

10 files changed

Lines changed: 2694 additions & 0 deletions

‎docs/issues/1525-01-persistence-test-coverage.md‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
# Subissue Draft for #1525-01: Add DB Compatibility Matrix
2+
3+
## Goal
4+
5+
Establish a compatibility matrix that exercises persistence-layer tests across supported database
6+
versions before any refactoring begins.
7+
8+
## Why First
9+
10+
The later refactors change persistence architecture, async behavior, schema setup, and backend
11+
implementations. Running the tests against multiple database versions first gives a baseline to
12+
detect regressions early and narrows review scope to behavior rather than guesswork.
13+
14+
## Scope
15+
16+
- Bash is acceptable for low-complexity orchestration.
17+
- Focus only on the database compatibility matrix; end-to-end real-client testing is covered by
18+
subissue #1525-02.
19+
20+
## Testing Principles
21+
22+
The implementation must follow these quality rules for all new and modified tests.
23+
24+
- **Isolation**: Each test run must be independent. Tests that spin up database containers via
25+
`testcontainers` already get their own ephemeral container; the bash matrix script achieves
26+
isolation by running one matrix cell at a time in a fresh process, each with an exclusively
27+
allocated container.
28+
- **Independent system resources**: Tests must not hard-code host ports. `testcontainers` binds
29+
containers to random free host ports automatically — do not override this with fixed bindings.
30+
Temporary files or directories, if needed, must be created under a `tempfile`-managed path so
31+
they are always removed on exit.
32+
- **Cleanup**: After each test (success or failure) all containers, volumes, and temporary files
33+
must be released. `testcontainers` handles containers automatically when the handle is dropped;
34+
ensure `Drop` is not suppressed.
35+
- **Behavior, not implementation**: Tests must assert observable outcomes (e.g. the driver
36+
correctly inserts and retrieves a torrent entry) rather than internal state (e.g. a specific SQL
37+
query was issued).
38+
- **Verified before done**: No test is considered complete until it has been executed and passes
39+
in a clean environment. Include confirmation of a passing run in the PR description.
40+
41+
## Reference QA Workflow
42+
43+
The PR #1695 review branch includes a QA script that defines the expected behavior:
44+
45+
- `run-db-compatibility-matrix.sh`:
46+
executes a compatibility matrix across SQLite, multiple MySQL versions, and multiple PostgreSQL
47+
versions.
48+
49+
This should be treated as a reference prototype, not a production artifact. The goal is to
50+
re-implement it in a form that integrates with the repository's normal test strategy.
51+
52+
## Dependency Note
53+
54+
PostgreSQL is not implemented yet, so this subissue cannot require successful execution against
55+
PostgreSQL. The structure should make it easy to add PostgreSQL combinations in subissue
56+
`#1525-08` once the driver exists.
57+
58+
## Proposed Branch
59+
60+
- `1525-01-db-compatibility-matrix`
61+
62+
## Tasks
63+
64+
### 1) Port the compatibility matrix workflow
65+
66+
Add a low-complexity bash compatibility-matrix runner that exercises persistence-related tests
67+
across supported database versions.
68+
69+
Tests to orchestrate:
70+
71+
- `cargo check --workspace --all-targets`
72+
- configuration coverage for PostgreSQL connection settings
73+
- large-download counter saturation tests in the HTTP protocol layer
74+
- large-download counter saturation tests in the UDP protocol layer
75+
- SQLite driver tests
76+
- MySQL driver tests across selected MySQL versions
77+
78+
Note: PostgreSQL version-matrix execution is deferred to subissue #1525-08, once the
79+
PostgreSQL driver exists.
80+
81+
Steps:
82+
83+
- Modify current DB driver tests so the DB image version can be injected through environment
84+
variables:
85+
- MySQL: `TORRUST_TRACKER_CORE_MYSQL_DRIVER_IMAGE_TAG`
86+
- PostgreSQL (reserved for subissue #1525-08): `TORRUST_TRACKER_CORE_POSTGRES_DRIVER_IMAGE_TAG`
87+
88+
When `TORRUST_TRACKER_CORE_MYSQL_DRIVER_IMAGE_TAG` is not set, the test falls back to the
89+
current hardcoded default (e.g. `8.0`), preserving existing behavior. The matrix script sets
90+
this variable explicitly for each version in the loop, so unset means "run as today" and the
91+
matrix just expands that into multiple combinations.
92+
93+
- Add `contrib/dev-tools/qa/run-db-compatibility-matrix.sh` modeled after the PR prototype:
94+
- `set -euo pipefail`
95+
- define default version sets from env vars:
96+
- `MYSQL_VERSIONS` defaulting to at least `8.0 8.4`
97+
- `POSTGRES_VERSIONS` reserved for subissue #1525-08
98+
- run pre-checks once (`cargo check --workspace --all-targets`)
99+
- run protocol/configuration tests once
100+
- run SQLite driver tests once
101+
- loop MySQL versions: `docker pull mysql:<version>`, then run MySQL driver tests with
102+
`TORRUST_TRACKER_CORE_RUN_MYSQL_DRIVER_TEST=1` and
103+
`TORRUST_TRACKER_CORE_MYSQL_DRIVER_IMAGE_TAG=<version>`
104+
- print a clear heading for each backend/version before executing tests
105+
- fail fast on first failure with the failing backend/version visible in logs
106+
- keep script complexity intentionally low; avoid re-implementing test logic already in test
107+
functions
108+
- Replace the current single MySQL `database` step in `.github/workflows/testing.yaml` with
109+
execution of the new script.
110+
111+
Acceptance criteria:
112+
113+
- [ ] DB image version injection is supported via `TORRUST_TRACKER_CORE_MYSQL_DRIVER_IMAGE_TAG`
114+
(and a reserved `POSTGRES` equivalent for subissue #1525-08).
115+
- [ ] `contrib/dev-tools/qa/run-db-compatibility-matrix.sh` exists and runs successfully.
116+
- [ ] The script exercises SQLite and at least two MySQL versions by default.
117+
- [ ] Failures identify the backend/version combination that broke.
118+
- [ ] The `database` job step in `.github/workflows/testing.yaml` runs the matrix script instead
119+
of a single-version MySQL command.
120+
- [ ] The script structure allows PostgreSQL to be added in subissue #1525-08 without a redesign.
121+
- [ ] Tests do not hard-code host ports; `testcontainers` assigns random ports automatically.
122+
- [ ] All containers started by tests are removed unconditionally on test completion or failure.
123+
124+
### 2) Document the workflow
125+
126+
Steps:
127+
128+
- Document the local invocation command for the matrix script.
129+
- Document that the CI `database` step runs the same script.
130+
131+
Acceptance criteria:
132+
133+
- [ ] The matrix script is documented and runnable without ad hoc manual steps.
134+
135+
## Out of Scope
136+
137+
- qBittorrent end-to-end testing (covered by subissue #1525-02).
138+
- Adding PostgreSQL support itself.
139+
- Refactoring the production persistence interfaces.
140+
- Performance benchmarking, before/after comparison, and benchmark reporting.
141+
142+
## Definition of Done
143+
144+
- [ ] `cargo test --workspace --all-targets` passes.
145+
- [ ] `linter all` exits with code `0`.
146+
- [ ] The matrix script has been executed successfully in a clean environment; a passing run log
147+
is included in the PR description.
148+
149+
## References
150+
151+
- EPIC: #1525
152+
- Reference PR: #1695
153+
- Reference implementation branch: `josecelano:pr-1684-review` — see EPIC for checkout
154+
instructions (`docs/issues/1525-overhaul-persistence.md`)
155+
- Reference script: `contrib/dev-tools/qa/run-db-compatibility-matrix.sh`

‎docs/issues/1525-02-qbittorrent-e2e.md‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
# Subissue Draft for #1525-02: Add qBittorrent End-to-End Test
2+
3+
## Goal
4+
5+
Add a high-level end-to-end test that validates tracker behavior through a complete torrent-sharing
6+
scenario using real containerized BitTorrent clients, covering scenarios that lower-level unit and
7+
integration tests cannot reach.
8+
9+
## Why Before the Refactor
10+
11+
The persistence refactor changes storage behavior underneath the tracker. Having a real-client
12+
scenario that exercises a full download cycle (seeder uploads → leecher downloads → tracker
13+
records completion) gives a regression backstop that is not possible with protocol-level tests
14+
alone.
15+
16+
## Scope
17+
18+
- Follow the same pattern as the existing `e2e_tests_runner` binary
19+
(`src/console/ci/e2e/runner.rs`): a Rust binary that drives the whole scenario using
20+
`std::process::Command` to invoke `docker compose` and any container-side commands.
21+
- Use SQLite as the database backend; database compatibility across multiple versions is already
22+
covered by subissue #1525-01.
23+
- Cover one complete scenario: a seeder sharing a torrent that a leecher downloads in full.
24+
- The binary is responsible for scaffolding (generating a temporary config and torrent file),
25+
starting the services, sending commands into the qBittorrent containers (via their WebUI API
26+
or `docker exec`), polling for completion, asserting the result, and tearing down.
27+
- Do not re-test things already covered at a lower level: announce parsing, scrape format,
28+
whitelist/key logic, or multi-database compatibility.
29+
30+
## Testing Principles
31+
32+
The implementation must follow these quality rules.
33+
34+
- **Isolation**: Each run of the E2E binary must be isolated from any other concurrently running
35+
instance. Achieve this by using a unique Docker Compose project name per run (e.g.
36+
`--project-name qbt-e2e-<random-suffix>`) so container names, networks, and volumes never
37+
collide with a parallel run.
38+
- **Independent system resources**: Do not bind services to fixed host ports. Let Docker assign
39+
ephemeral host ports and discover them from the compose output, so two simultaneous runs cannot
40+
conflict. Place all temporary files (tracker config, payload, `.torrent` file) in a
41+
`tempfile`-managed directory created at runner start and deleted on exit.
42+
- **Cleanup**: `docker compose down --volumes` must be called unconditionally — on success, on
43+
assertion failure, and on panic. Use a Rust `Drop` guard or equivalent to guarantee teardown
44+
even when the runner exits unexpectedly.
45+
- **Mock time when possible**: Use a configurable timeout (CLI argument or env var) for the
46+
leecher-completion poll rather than a hard-coded sleep. If any logic depends on wall-clock time
47+
(e.g. stale peer detection), inject a mockable clock consistent with the `clock` package used
48+
elsewhere in the codebase.
49+
- **Behavior, not implementation**: Assert the outcome the user cares about — the leecher holds a
50+
complete, byte-identical copy of the payload — not which internal tracker counters changed or
51+
which announce endpoints were called.
52+
- **Verified before done**: The binary must be executed end-to-end and produce a passing result in
53+
a clean environment before the subissue is closed. Include a run log in the PR description.
54+
55+
## Reference QA Workflow
56+
57+
`contrib/dev-tools/qa/run-qbittorrent-e2e.py` in the PR #1695 review branch demonstrates the
58+
scenario (seeder + leecher + tracker via Python subprocess). Treat it as a behavioral reference
59+
only; the implementation here will use `docker compose` instead of manual container management.
60+
61+
## Proposed Branch
62+
63+
- `1525-02-qbittorrent-e2e`
64+
65+
## Tasks
66+
67+
### 1) Add a docker compose file for the E2E scenario
68+
69+
Add a compose file (e.g., `compose.qbittorrent-e2e.yaml`) that defines:
70+
71+
- the tracker service configured with SQLite
72+
- a qbittorrent-seeder container
73+
- a qbittorrent-leecher container
74+
75+
Steps:
76+
77+
- Define a tracker service mounting a SQLite config file (generated by the runner).
78+
- Define seeder and leecher services using a suitable qBittorrent image.
79+
- Configure a shared network so all containers can reach each other and the tracker.
80+
- Define any volumes needed to mount the payload and torrent file into each client container.
81+
- Ensure `docker compose up --wait` exits cleanly when services are healthy.
82+
- Ensure `docker compose down --volumes` removes all containers and volumes.
83+
84+
Acceptance criteria:
85+
86+
- [ ] `docker compose -f compose.qbittorrent-e2e.yaml up --wait` starts all services without error.
87+
- [ ] `docker compose -f compose.qbittorrent-e2e.yaml down --volumes` leaves no orphaned resources.
88+
89+
### 2) Implement the Rust runner binary
90+
91+
Add a new binary (e.g., `src/bin/qbittorrent_e2e_runner.rs`) that follows the same structure as
92+
`src/console/ci/e2e/runner.rs`:
93+
94+
- Parses CLI arguments or environment variables (compose file path, payload size, timeout).
95+
- Generates scaffolding: a temporary tracker config (SQLite) and a small deterministic payload
96+
with its `.torrent` file.
97+
- Calls `docker compose up` via `std::process::Command`.
98+
- Seeds the payload: injects the torrent and payload into the seeder container via the qBittorrent
99+
WebUI REST API (or `docker exec` as a fallback) and starts seeding.
100+
- Leaches the payload: injects the `.torrent` file into the leecher container and starts
101+
downloading.
102+
- Polls for completion: queries the leecher's WebUI API until the torrent state reaches
103+
`uploading` (100 % downloaded) or a timeout expires.
104+
- Asserts payload integrity: compares the downloaded file against the original (hash or byte
105+
comparison).
106+
- Calls `docker compose down --volumes` unconditionally (even on assertion failure), mirroring
107+
the cleanup pattern in `tracker_container.rs`.
108+
109+
Steps:
110+
111+
- Add a shared `docker compose` wrapper at `src/console/ci/compose.rs` (see below). This
112+
module is not specific to qBittorrent and is reused by the benchmark runner in subissue
113+
`#1525-03`.
114+
- Add a `qbittorrent` module under `src/console/ci/` (parallel to `e2e/`) containing:
115+
- `runner.rs` — main orchestration logic
116+
- `qbittorrent_client.rs` — HTTP calls to the qBittorrent WebUI API
117+
- **`src/console/ci/compose.rs` wrapper** — mirrors `docker.rs` but targets `docker compose`
118+
subcommands. Design it around a `DockerCompose` struct that holds the compose file path and
119+
project name:
120+
- `DockerCompose::new(file: &Path, project: &str) -> Self`
121+
- `up(&self) -> io::Result<()>` — runs `docker compose -f <file> -p <project> up --wait --detach`
122+
- `down(&self) -> io::Result<()>` — runs `docker compose -f <file> -p <project> down --volumes`
123+
- `port(&self, service: &str, container_port: u16) -> io::Result<u16>` — runs
124+
`docker compose -f <file> -p <project> port <service> <port>` and parses the host port so
125+
the runner never hard-codes ports
126+
- `exec(&self, service: &str, cmd: &[&str]) -> io::Result<Output>` — wraps
127+
`docker compose -f <file> -p <project> exec <service> <cmd…>` for injecting commands into
128+
running containers
129+
- Implement `Drop` on a `RunningCompose` guard returned by `up` that calls `down`
130+
unconditionally, matching the `RunningContainer::drop` pattern in `docker.rs`
131+
- Use `tracing` for progress output consistent with the rest of the runner
132+
- Generate a fixed small payload (e.g., 1 MiB of deterministic bytes) at runtime; store the
133+
`.torrent` file in a `tempfile` directory so it is cleaned up automatically.
134+
- Re-use `tracing` for progress output, consistent with the existing runner.
135+
136+
Acceptance criteria:
137+
138+
- [ ] The runner completes a full seeder → leecher download using the containerized tracker.
139+
- [ ] Payload integrity is verified after download (hash or byte comparison).
140+
- [ ] The runner can be executed repeatedly without manual setup or teardown.
141+
- [ ] No orphaned containers or volumes remain on success or failure.
142+
- [ ] The binary is documented in the top-level module doc comment with an example invocation.
143+
- [ ] Each invocation uses a unique compose project name so parallel runs do not conflict.
144+
- [ ] All temporary files are placed in a managed temp directory and deleted on exit.
145+
- [ ] No fixed host ports are used; ports are discovered dynamically from the compose output.
146+
- [ ] `docker compose down --volumes` is called unconditionally via a `Drop` guard.
147+
148+
### 3) Document the E2E workflow
149+
150+
Steps:
151+
152+
- Document the local invocation command (e.g., `cargo run --bin qbittorrent_e2e_runner`).
153+
- Document any prerequisites (Docker, image availability, open ports).
154+
- Clarify that this test is not run in the standard `cargo test` suite due to resource
155+
requirements and describe how it is triggered in CI (opt-in env var or separate job).
156+
157+
Acceptance criteria:
158+
159+
- [ ] The test is documented and runnable without ad hoc manual steps.
160+
161+
## Out of Scope
162+
163+
- Testing multiple database backends (covered by subissue #1525-01).
164+
- Testing announce or scrape protocol correctness at the protocol level.
165+
- UDP tracker E2E (can be added later without redesigning the compose setup).
166+
167+
## Definition of Done
168+
169+
- [ ] `cargo test --workspace --all-targets` passes (or the E2E test is explicitly excluded with a
170+
documented opt-in flag).
171+
- [ ] `linter all` exits with code `0`.
172+
- [ ] The E2E runner has been executed successfully in a clean environment; a passing run log is
173+
included in the PR description.
174+
175+
## References
176+
177+
- EPIC: #1525
178+
- Reference PR: #1695
179+
- Reference implementation branch: `josecelano:pr-1684-review` — see EPIC for checkout
180+
instructions (`docs/issues/1525-overhaul-persistence.md`)
181+
- Reference script: `contrib/dev-tools/qa/run-qbittorrent-e2e.py`
182+
- Existing runner pattern: `src/console/ci/e2e/runner.rs`
183+
- Docker command wrapper: `src/console/ci/e2e/docker.rs`
184+
- Existing container wrapper patterns: `src/console/ci/e2e/tracker_container.rs`

0 commit comments

Comments
 (0)