Skip to content

Improve backend tests #758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tegioz merged 1 commit into from
Oct 28, 2026
Merged

Improve backend tests #758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions docs/backend.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,20 +307,27 @@ surface per layer:
recipients, `template_data` content, and the tracking record. Callers,
including handler tests, assert only kind, recipients, and identifiers.
- Error contract (`handlers/error/tests.rs`): no mocks; asserts each
`HandlerError` variant's status and body.
`HandlerError` variant's status and body, and the status and body each
typed manager error kind (`OCG01` rejection, `Rejected`, internal failure)
maps to through its `From` impl.
- Contract (`db/contract_tests`, `just db-contract-tests`): real database;
asserts SQL JSON to Rust DTO shapes, the `PgUnitOfWork` lifecycle
(rollback on drop, explicit rollback, failing deferred constraint on
commit), and manager compositions that mocks cannot prove.
asserts SQL JSON to Rust DTO shapes, lock ordering, the `PgUnitOfWork`
lifecycle (rollback on drop, explicit rollback, failing deferred
constraint on commit), that an `OCG01` rejection reaches Rust as a
user-facing error, and manager compositions that mocks cannot prove. A
test that only re-asserts a pgTAP `throws_ok` rejection is not repeated
here; a rejection is asserted only as the outcome of a race, lifecycle, or
composition the test exists to prove, or to pin how a wrapper passes its
arguments.

Rules that follow from the layering:

- Handler tests build the router with `TestRouterBuilder` and the shared
session and permission helpers. A handler behind a manager has one or two
manager expectations per test and no workflow `MockDB` expectations. Every
typed manager error is proven at the handler with three tests: an `OCG01`
rejection returns 422 with its message, a `Rejected` returns 422 with its
message, and an internal failure returns 500 with an empty body.
manager expectations per test and no workflow `MockDB` expectations. The
mapping of typed manager errors to responses is proven once by the error
contract tests; each route that calls a manager has one test showing that
a manager rejection returns 422 with its message.
- Handler DB-failure tests exist only when they prove something the error
contract tests cannot: a failure mapped to a non-500 status, a suppressed
later side effect (asserted with `.never()` on the side effect and its
Expand All @@ -330,16 +337,24 @@ Rules that follow from the layering:
- Shared sample builders for domain types live in `types/tests.rs`;
`handlers/tests.rs` re-exports them, and `services` or `templates` tests
import them from `types::tests`, so test code follows the same dependency
direction as production code.
direction as production code. Other shared helpers live next to what they
build: `MockDB` expectation helpers in `db::mock`, configuration samples in
`config::tests`, router, request, session, and permission helpers in
`handlers/tests.rs`, and helpers that belong to no layer (byte fixtures,
text normalization, polling) in `test_util`.
- A test file's local helpers share what is identical and incidental across
its tests and keep visible what each test exists to prove. Request
builders for a route and the fixed set of context reads a page always
performs belong in a helper; a read whose return value drives the scenario
is passed to the helper as an explicit argument, never selected by a flag,
and is written inline when only one test needs it. Assertions on the
response (status, headers, body) stay inline in every test.
- Pure functions and value logic are tested with tables: one test iterates
labeled cases and every assertion message names the case. Handler and
manager tests keep one test per scenario.
- A test is removed only when the PR description names the test that proves
the same behavior at another layer. A test with no replacement stays.
the same behavior, at the same or another layer. A test with no
replacement stays.

## Bounded operations and worker health

Expand Down
9 changes: 6 additions & 3 deletions docs/database.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,12 @@ assertions, so the seeded rows also prove they survive later migrations.
`ocg-server/src/db/contract_tests/` runs the ignored `db_contracts` tests
against a real database migrated and seeded with `tests/data/contract.sql`.
They guard the JSON boundary between functions and Rust DTOs, lock ordering,
and the error SQLSTATEs, with one module per database trait. A change to a
JSON-returning function, a DTO, a wrapper, or `contract.sql` runs
`just db-contract-tests`.
and that an `OCG01` rejection reaches Rust as a user-facing error, with one
module per database trait. A test that only re-asserts a pgTAP `throws_ok`
rejection is not repeated in Rust; a rejection is asserted here only as the
outcome of a race, lifecycle, or composition the test exists to prove, or to
pin how a wrapper passes its arguments. A change to a JSON-returning
function, a DTO, a wrapper, or `contract.sql` runs `just db-contract-tests`.

### Commands

Expand Down
137 changes: 54 additions & 83 deletions ocg-common/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,31 +105,6 @@ mod tests {

use super::*;

#[test]
fn test_database_config_accepts_default_tls_configuration() {
let db = sample_db_config();

assert!(db.validate().is_ok());
}

#[test]
fn test_database_config_accepts_verification_with_required_tls() {
// Setup full certificate verification over required TLS
let mut db = sample_db_config();
db.connection.ssl_mode = Some(DeadpoolSslMode::Require);
db.tls = Some(DbTlsConfig {
mode: DbTlsMode::VerifyFull,

ca_cert: Some("trusted CA certificate".to_string()),
});

// Validate the database transport configuration
let result = db.validate();

// Check secure TLS configuration is accepted
assert!(result.is_ok());
}

#[test]
fn test_database_config_deserializes_tls_fields() {
// Setup the chart-compatible flattened database configuration
Expand All @@ -153,70 +128,66 @@ tls:
}

#[test]
fn test_database_config_rejects_ca_certificate_without_verification() {
// Setup CA material that certificate verification would ignore
let mut db = sample_db_config();
db.tls = Some(DbTlsConfig {
mode: DbTlsMode::None,

ca_cert: Some("unused CA certificate".to_string()),
});

// Validate the database transport configuration
let result = db.validate();

// Check ignored trust material is rejected
assert_eq!(
result.unwrap_err().to_string(),
"db.tls.ca_cert requires db.tls.mode verify-ca or verify-full"
);
}

#[test]
fn test_database_config_rejects_verification_without_ca_certificate() {
// Setup certificate verification without trust roots
let mut db = sample_db_config();
db.connection.ssl_mode = Some(DeadpoolSslMode::Require);
db.tls = Some(DbTlsConfig {
mode: DbTlsMode::VerifyFull,

ca_cert: None,
});

// Validate the database transport configuration
let result = db.validate();

// Check missing trust roots are rejected
assert_eq!(
result.unwrap_err().to_string(),
"db.tls.ca_cert is required when db.tls.mode enables certificate verification"
);
}

#[test]
fn test_database_config_rejects_verification_without_required_tls() {
for ssl_mode in [
None,
Some(DeadpoolSslMode::Disable),
Some(DeadpoolSslMode::Prefer),
] {
// Setup certificate verification with a plaintext-capable SSL mode
fn test_database_config_validate_checks_tls_policy() {
let require = Some(DeadpoolSslMode::Require);
let ca_cert = Some("trusted CA certificate");
let verification_without_tls =
"db.ssl_mode must be Require when db.tls.mode enables certificate verification";
// (case, ssl_mode, tls mode and CA certificate, expected error)
let cases = [
("default configuration", None, None, None),
(
"full verification over required TLS",
require,
Some((DbTlsMode::VerifyFull, ca_cert)),
None,
),
(
"CA certificate without verification",
None,
Some((DbTlsMode::None, Some("unused CA certificate"))),
Some("db.tls.ca_cert requires db.tls.mode verify-ca or verify-full"),
),
(
"verification without CA certificate",
require,
Some((DbTlsMode::VerifyFull, None)),
Some(
"db.tls.ca_cert is required when db.tls.mode enables certificate verification",
),
),
(
"verification without ssl mode",
None,
Some((DbTlsMode::VerifyCa, ca_cert)),
Some(verification_without_tls),
),
(
"verification with disabled ssl",
Some(DeadpoolSslMode::Disable),
Some((DbTlsMode::VerifyCa, ca_cert)),
Some(verification_without_tls),
),
(
"verification with preferred ssl",
Some(DeadpoolSslMode::Prefer),
Some((DbTlsMode::VerifyCa, ca_cert)),
Some(verification_without_tls),
),
];

for (case, ssl_mode, tls, expected) in cases {
let mut db = sample_db_config();
db.connection.ssl_mode = ssl_mode;
db.tls = Some(DbTlsConfig {
mode: DbTlsMode::VerifyCa,
db.tls = tls.map(|(mode, ca_cert)| DbTlsConfig {
mode,

ca_cert: Some("trusted CA certificate".to_string()),
ca_cert: ca_cert.map(str::to_string),
});

// Validate the database transport configuration
let result = db.validate();
let result = db.validate().map_err(|err| err.to_string());

// Check every plaintext-capable mode is rejected
assert_eq!(
result.unwrap_err().to_string(),
"db.ssl_mode must be Require when db.tls.mode enables certificate verification"
);
assert_eq!(result.err().as_deref(), expected, "{case}");
}
}

Expand Down
Loading
Loading