Skip to content

CI runs no canary invariant tests — tests/test_canary_invariants.py is outside tests/unit/ #2037

Description

@obasilakis

Problem

tests/test_canary_invariants.py — ~126 tests covering the whole canary invariant
library (S-01/S-02/S-03, E-01…E-06, G-03/G-04, B-01/B-02, L-03, R-01, plus the
CanaryAlerts Slack payload/emit path) — is executed by no CI workflow.

Both unit workflows run only tests/unit/:

  • .github/workflows/backend-unit-test.yml → cd tests && python -m pytest unit/ -m "not slow" …
  • .github/workflows/backend-unit-nightly.yml → same invocation, base vs head

The file lives at tests/ root, so it is outside that path. It is a pure unit
file (synthetic Snapshot dataclasses + a FakeRedis; no live Redis, no live
instance), so nothing about it requires the integration tiers — it is simply in
the wrong directory.

Surfaced during review of #2022 (ent#335/336/337). That PR put its new tests in
tests/unit/ for exactly this reason, which is right for the new coverage but
leaves the ~126 pre-existing canary tests unrun: a regression in the older
S-03 / R-01 / E-06 / L-03 assertions goes undetected in CI.

Evidence that this is already costing us

Running the file locally today:

$ cd tests && python -m pytest test_canary_invariants.py -q
125 passed, 1 skipped, 19 errors

The 19 errors are all setup errors on TestCanaryService / TestCanarySlackPayload
/ TestCanarySlackEmit:

ModuleNotFoundError: No module named 'utils.safe_yaml'
  src/backend/services/template_service.py:22

i.e. the reload_canary fixture's sys.modules surgery leaves utils shadowed,
and #1965/#1978's new utils.safe_yaml import cannot resolve through it.
Reproduces identically on dev (e6df5bf8), so it is pre-existing and unrelated
to any one PR — it is exactly the kind of silent decay that a workflow running
this file would have caught the day it landed.

Fix

Two options, either is one small change:

  1. Move the file to tests/unit/test_canary_invariants.py (matches where
    fix(canary): stop three false-positive classes drowning the harness (ent#335/336/337) #2022's test_ent33* files went, and where the rest of the canary coverage
    now lives). Needs the 19 fixture errors above fixed in the same PR, or CI goes
    red on merge.
  2. Add it to the workflow invocation explicitly, same caveat.

Option 1 is preferred — it removes the class of problem rather than one instance,
and keeps "unit tests live in tests/unit/" true.

⚠️ Note the same gap likely applies to other pure-unit files at tests/ root
(there are ~109 tests/test_*.py, most of which need a live instance and
legitimately belong to the integration tiers — but not all). Worth a quick pass
to see which others are unit-only and equally unrun; out of scope for the
one-line fix above.

Acceptance criteria

  1. tests/test_canary_invariants.py's assertions execute in backend-unit-test.yml
    on every PR.
  2. The 19 utils.safe_yaml setup errors are resolved (not xfail-ed) so the file
    is green and a future regression is a visible red.
  3. A note in tests/README.md stating that pure-unit files belong in tests/unit/
    because that is the only path CI runs.

Refs

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

    complexity-mediumComplexity: medium (board points 5-8)priority-p2Importantstatus-readyGreenlit and ready for development (vetted; counterpart to status-incubating)theme-devexTheme: DevExtype-bugBug fix

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions