Skip to content

system_cleanup queue-metadata cleanup is not unit-testable without Redis (add Redis to CI or refactor to be generic) #830

Description

@roeezolantz

Summary

Follow-up to #816 (fix for #815). The system_cleanup handler that prunes Redis
queue metadata currently has ~0% patch coverage on its core branch, and there is
no straightforward way to unit-test it today. This issue tracks making that code
path testable.

Background

#816 changed handle_cleanup_request in
src/jobs/handlers/system_cleanup_handler.rs to source the Redis pool from the
queue backend instead of the transaction repository, so cleanup runs whenever
the queue backend is Redis regardless of REPOSITORY_STORAGE_TYPE. Codecov
flagged the changed lines as uncovered.

Why it's hard to cover right now

All of the changed lines sit behind the backend_type() == Redis guard, and the
only way to obtain a non-None redis_connections() is through Queue::setup(),
which opens live Redis ConnectionManagers. Consequently:

  • A plain unit test cannot reach those lines without a running Redis.
  • CI has no Redis service in the coverage job (.github/workflows/ci.yaml), and
    the existing *_redis.rs tests are all #[ignore]d, so a Redis-gated
    integration test would not run in CI (and thus wouldn't contribute coverage).
  • handle_cleanup_request takes a concrete ThinData<DefaultAppState> (hard-wired
    to the concrete JobProducer), so a MockJobProducerTrait cannot be injected
    as-is.

Proposed options (any one, or a combination)

  1. Make the inner handler generic over the job producer, mirroring the existing
    relayer_health_check_handler pattern (public concrete wrapper +
    generic _impl). This allows injecting MockJobProducerTrait and unit-testing
    the two skip branches (non-Redis backend; Redis backend with no connections)
    in normal CI — covers most of the diff without any infra.

  2. Add a Redis service to the coverage CI job and either un-#[ignore] a
    focused integration test or add a new one that exercises the full
    prune path (<queue>:{done,failed,dead} + :data/:data::result).

  3. Add a Redis-gated #[ignore]d integration test (mirrors the current
    *_redis.rs convention) that documents/verifies the end-to-end behavior even
    if it doesn't count toward CI coverage.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions