Skip to content

Improve Whitaker test-helper quality - #334

Draft
leynos wants to merge 3 commits into
fix-audit-vulnerabilitiesfrom
test-quality-sweep
Draft

Improve Whitaker test-helper quality#334
leynos wants to merge 3 commits into
fix-audit-vulnerabilitiesfrom
test-quality-sweep

Conversation

@leynos

@leynos leynos commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Pull Request

Summary

This branch makes test-only assertion boundaries explicit for Whitaker and
removes test quality debt without changing runtime behaviour.

  • Adds the shared ExpectValid assertion boundary for test-only helpers.
  • Routes helper assertions through that boundary so Whitaker accepts intentional
    test failures.
  • Upgrades the CI Whitaker installer to 0.2.7 for current suite behaviour.
  • Parameterizes equivalent heartbeat and secret-masking cases, and removes
    tautological no-panic tests that only restated their premises.
  • Moves the legacy libSQL migration schema into a named SQL fixture.
  • Opens #333 for the separately
    scoped in-process environment-mutation migration and its Clippy guardrail.

Change Type

  • CI/Infrastructure
  • Refactor
  • Tests

Linked Issue

None; this is a targeted CI lint-regression repair and test-quality sweep.

Review walkthrough

Validation

  • make check-fmt
  • make lint
  • make typecheck
  • make test — 4,247 passed, 8 skipped; GitHub tool tests: 5 passed
  • git diff --check

Security Impact

None. This changes test assertion boundaries, test structure, fixture placement,
and CI lint tooling only.

Database Impact

None. The moved SQL fixture preserves the legacy schema used by the existing
libSQL migration test; it adds no runtime schema, migration, query, or data
change.

Blast Radius

Limited to test helpers compiled by the Whitaker CI lint job and test-source
organization.

Rollback Plan

Revert the shared assertion boundary, test-quality commit, and installer update
if they prove incompatible.


Review track: C (CI)

References

Summary by Sourcery

Improve Whitaker compatibility and test quality without changing runtime behavior.

Enhancements:

  • Introduce a shared test-only assertion boundary so Whitaker can distinguish intentional helper failures from production assertions.
  • Improve test maintainability by consolidating equivalent heartbeat and secret-masking cases and removing tautological no-panic coverage.
  • Move the legacy libSQL migration schema into a named SQL fixture without changing migration behavior.

CI:

  • Upgrade the Whitaker CI installer to version 0.2.7.

Tests:

  • Update test helpers and fixtures across the suite to use the explicit assertion boundary.

leynos added 3 commits August 23, 2026 04:00
Use Whitaker installer 0.2.7 so CI recognises helpers inside test-only
modules and reports genuine lint violations rather than false positives.
Replace helper-level `expect` calls with the shared `ExpectValid` assertion
boundary so Whitaker can distinguish intentional test failures from
production error handling.
Parameterize equivalent cases, remove tests that only restate their
premises, and move the legacy libSQL schema into a named fixture. Keep
assertions focused on observable behaviour and make migration input easier
to inspect independently.
@github-actions github-actions Bot added scope: channel/cli TUI / CLI channel scope: channel/web Web gateway channel scope: tool/builtin Built-in tools scope: llm LLM integration scope: orchestrator Container orchestrator scope: worker Container worker scope: ci CI/CD workflows scope: agent Agent core (agent loop, router, scheduler) scope: channel/wasm WASM channel runtime scope: safety Prompt injection defense scope: setup Onboarding / setup labels Aug 23, 2026
@sourcery-ai

sourcery-ai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Introduces a shared ExpectValid assertion boundary for tests, updates test helpers and cases to use it, consolidates repetitive test cases with rstest, upgrades the Whitaker CI installer version, and moves a legacy libSQL schema into a reusable SQL fixture without changing runtime behaviour.

Flow diagram for the legacy libSQL test fixture

flowchart LR
    LegacySchema[legacy_wasm_wit_schema.sql]
    MigrationTest[libsql_wit_defaults migration test]
    MigrationLogic[Existing migration logic]

    LegacySchema --> MigrationTest
    MigrationTest --> MigrationLogic
Loading

File-Level Changes

Change Details Files
Introduce and wire the shared ExpectValid assertion boundary across test-only code so Whitaker treats these panics as explicit test assertions.
  • Add ExpectValid trait in test_support with implementations for Result and Option that panic with a formatted message
  • Replace direct .expect(...) calls with .expect_valid(...) in test helpers and tests across dispatcher, channels, skills, worker, bootstrap, history, tools, and other modules
  • Tighten some generic bounds (e.g., add Display to error types) to support ExpectValid formatting
src/test_support.rs
src/agent/dispatcher/tests/auth.rs
src/agent/dispatcher/tests/image_sentinel.rs
src/agent/dispatcher/tests/loop_guard.rs
src/agent/dispatcher/tests/pipeline.rs
src/agent/dispatcher/tests/skill_bundle_context_bdd.rs
src/agent/dispatcher/tests/skills.rs
src/agent/scheduler/tests/approval.rs
src/agent/thread_ops/document_store/tests/mod.rs
src/bootstrap/tests/env_format.rs
src/bootstrap/tests/migration.rs
src/bootstrap/tests/migration_support.rs
src/channels/wasm/wrapper/tests/channel/typing.rs
src/channels/wasm/wrapper/tests/dispatch.rs
src/channels/web/handlers/skills/tests/helpers.rs
src/channels/web/handlers/skills/tests/multipart.rs
src/channels/web/server/tests/fixtures.rs
src/channels/web/server/tests/oauth.rs
src/history/migrations/tests/postgres_testing.rs
src/llm/rig_adapter/tests/unsupported_params.rs
src/orchestrator/api/tests/fixtures/remote_tool_helpers.rs
src/tools/builtin/skill_tools/tests/read_file_adapter.rs
src/tools/schema_validator/tests/fixture_groups.rs
src/worker/api/tests/client_methods.rs
src/worker/api/tests/transport_types.rs
src/worker/claude_bridge/tests/claude_fs_setup.rs
src/worker/container/tests/hosted_fidelity.rs
src/worker/container/tests/pre_loop.rs
src/worker/container/tests/remote_tools.rs
src/worker/container/tests/shutdown.rs
src/channels/web/server/tests/oauth.rs
src/channels/web/server/tests/fixtures.rs
src/channels/web/handlers/skills/tests/multipart.rs
src/channels/web/handlers/skills/tests/helpers.rs
Refactor tests to reduce duplication and improve structure using rstest and by removing tautological no-panic smoke tests.
  • Parameterize strip_html_comments and is_effectively_empty tests with rstest cases and shared constants
  • Parameterize mask_secret tests with additional edge cases and remove duplicate short-value tests
  • Remove doctor, prompts, runtime, and other tests that only asserted non-panicking behaviour or restated function premises
src/agent/heartbeat/tests.rs
src/safety/leak_detector/tests.rs
src/setup/prompts/tests.rs
src/app/tests.rs
src/cli/doctor/tests.rs
Adjust CI and test fixtures to match current Whitaker behaviour and centralize legacy SQL schema.
  • Bump WHITAKER_INSTALLER_VERSION from 0.2.6 to 0.2.7 in the code_style GitHub Actions workflow
  • Replace inline LEGACY_WASM_WIT_SCHEMA string in libsql_wit_defaults integration test with include_str! from a new SQL fixture file
  • Add tests/fixtures/libsql/legacy_wasm_wit_schema.sql containing the legacy wasm tools and channels schema
.github/workflows/code_style.yml
tests/db_integration/libsql_wit_defaults.rs
tests/fixtures/libsql/legacy_wasm_wit_schema.sql

Possibly linked issues

  • #Tests: improve helpers to avoid tempdir races, tautological assertions, and shape-only tests: The PR directly implements the issue’s test-quality improvements, including helper boundaries and removal of tautological no-op tests.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1a065f1c-7a51-4f98-b862-664655244c9b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate billing to continue.


Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size: XL 500+ changed lines label Aug 23, 2026
@github-actions github-actions Bot added risk: high Safety, secrets, auth, or critical infrastructure contributor: core 20+ merged PRs labels Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: high Safety, secrets, auth, or critical infrastructure scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: channel/web Web gateway channel scope: ci CI/CD workflows scope: llm LLM integration scope: orchestrator Container orchestrator scope: safety Prompt injection defense scope: setup Onboarding / setup scope: tool/builtin Built-in tools scope: worker Container worker size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant