Skip to content

ci: --features full does not include audit, so audit::storage tests never run in CI #26

@rrrodzilla

Description

@rrrodzilla

Summary

The Build & Test workflow runs two clippy + nextest matrices, both keyed on --features full:

```
cargo clippy -p acton-service --all-targets --features full -- -D warnings
cargo nextest run -p acton-service --features full
cargo clippy -p acton-service --all-targets --no-default-features --features "full,crypto-ring" -- -D warnings
cargo nextest run -p acton-service --no-default-features --features "full,crypto-ring"
```

In acton-service/Cargo.toml, full expands to:

```
full = ["http", "grpc", "websocket", "database", "cache", "events", "observability",
"resilience", "otel-metrics", "governor", "openapi", "cedar-authz", "jwt", "auth",
"session-memory", "session-redis", "htmx", "askama", "sse", "pagination-full",
"handlers", "login-lockout", "tls", "accounts", "account-handlers", "journald",
"graphql", "graphql-cedar"]
```

audit is not in the list. Neither are turso, surrealdb, or clickhouse. As a result:

  • Every test in acton-service::audit::storage::* (existing ClickHouse roundtrip coverage, new helper tests from fix(audit): warn when storage parser falls back to Custom for framework namespace #24, etc.) is gated out and never runs in CI.
  • All four storage backends compile only locally; CI never builds Turso, SurrealDB, or ClickHouse code paths.
  • The accounts/mod.rs audit integration (#[cfg(feature = "audit")] mod audit_integration) is never exercised.

Impact

This is the silent root cause of several bugs we've fixed in the past day:

If audit had been in the CI matrix from day one, several of these issues would have been caught at PR time instead of in production.

Suggested fix

Extend .github/workflows/build.yml with additional matrix entries covering the audit and storage-backend code paths. Concretely, add at minimum:

  1. full,audit — runs the existing audit module tests including all four storage-backend parser roundtrips.
  2. full,audit,turso — exercises Turso parser arms.
  3. full,audit,surrealdb — exercises SurrealDB parser arms.
  4. full,audit,clickhouse — exercises ClickHouse parser arms and the existing roundtrip coverage.

Some of these backends require running services (Postgres, ClickHouse, SurrealDB) for integration tests; pure unit tests on parsers and helpers don't, and those are the ones that catch the bugs above. A first cut can run unit tests only and skip integration-flavored tests via nextest filter expressions.

A separate, related decision: should full itself just include audit? The feature is meant to mean "everything most users care about," and audit is a first-class production capability. Including it would also surface a pre-existing test failure (extensions::tests::service_builder_without_actors_has_empty_extensions fails under --features "full,audit" on origin/main because of a tokio runtime mismatch in service_builder.rs:701) — but that's a bug to fix, not a reason to keep CI quiet about audit code.

Acceptance

  • CI matrix gains at least one entry that compiles and runs tests under --features audit (alone or combined with full).
  • CI matrix exercises each of the three optional storage backends (turso, surrealdb, clickhouse) at least at the unit-test / parser level.
  • Decide whether full should include audit going forward, and either fix the existing failing test or formally accept the divergence.

Out of scope

  • Integration testing of audit storage against live databases (those need service containers and are a bigger CI cost decision).
  • Fixing the pre-existing extensions::tests::service_builder_without_actors_has_empty_extensions failure — separate bug.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions