Skip to content

test(scenarios): CI never evaluates the scenarios' declared assertions #163

Description

@sepehr-safari

Nine scenario files declare an assertions block, and CI never evaluates any of
them. The corpus asserts things no test checks.

How the gap works

evaluateScenario() is what runs a scenario's declared assertions. Over the
registry it is reachable from exactly one place, ocpp-debugkit ci
(packages/toolkit/src/cli/commands/ci.ts:25-40), which loops scenarios and
evaluates each one.

.github/workflows/ci.yml never calls it. The steps are install, lint,
format:check, typecheck, build, test, plus build and
test:external-fixture in the second job. There is no ocpp-debugkit reference
in the workflow at all.

The only test touching evaluateScenario is
packages/toolkit/src/core/assertions.test.ts, and it builds its scenarios by
hand. It never imports the registry, so it proves the evaluator works without
proving any shipped scenario passes its own assertions.

Net effect: these nine files declare assertions that nothing verifies.

  • firmware-update-failure
  • firmware-update-success
  • heartbeat-irregular
  • heartbeat-timeout
  • meter-anomaly
  • refused-authorization
  • short-session
  • slow-csms-response
  • unresponsive-csms

packages/toolkit/src/scenarios/__scenarios__/unresponsive-csms.ts is the clearest
case: its failure_severity and failure_count assertions have never been run by
any automated check.

A second, related weakness

packages/toolkit/src/scenarios/index.test.ts checks detection with
expect(failures.some((f) => f.code === '...')).toBe(true). That is satisfied as
long as the expected code appears somewhere, so a scenario that also fires two
unrelated codes still passes. For a corpus whose whole purpose is pinning exact
detector behaviour, the assertion is weaker than the intent. Negative controls like
meter-value-zero (#108) depend on the absence of everything, which .some()
cannot express.

Fix

Preferred: add a vitest case that iterates the registry and evaluates every
scenario, so the assertions run inside the normal suite with no CLI dependency and
no workflow change:

for (const scenario of scenarios) {
  const result = evaluateScenario(scenario as Scenario);
  expect(result.allPassed, `${scenario.name}: ${JSON.stringify(result.assertions)}`).toBe(true);
}

Then tighten the per-scenario detection checks to compare the exact set of detected
codes against expectedFailures, rather than .some(). Expect that to surface
real disagreements on existing scenarios; each one is either a scenario bug or an
expectedFailures that was never complete, and both are worth knowing.

Adding ocpp-debugkit ci as a workflow step is the alternative. It reuses the
existing command, but it needs the built CLI and puts the check outside the suite,
so it is the weaker option.

Scope

Maintainer work, not a good-first-issue: the exact-set tightening is a judgement
call about what the corpus is allowed to assert, and it will likely require fixing
scenarios it exposes.

Pre-existing and unrelated to any current contribution. Found while reviewing #161.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions