Skip to content

chore: Write end to end tests for data commands#141

Merged
github-actions[bot] merged 1 commit intomainfrom
chore-e2e-testing
Feb 3, 2026
Merged

chore: Write end to end tests for data commands#141
github-actions[bot] merged 1 commit intomainfrom
chore-e2e-testing

Conversation

@stack72
Copy link
Copy Markdown
Contributor

@stack72 stack72 commented Feb 3, 2026

No description provided.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: chore: Write end to end tests for data commands

Summary

This PR adds comprehensive integration tests for the data output flow, testing the full lifecycle from definition creation through method execution to data verification. The tests are well-structured and provide good coverage of the CLI commands.

No Blocking Issues

The code is well-written and follows the project's conventions.


Code Quality Analysis

TypeScript Compliance:

  • No any types used - the code properly uses inline type annotations like { modelName: string } and { type: string } for dynamic data
  • All imports are named exports as required
  • Code follows the existing patterns established in other integration tests

Test Structure:

  • Tests are properly located in the integration/ directory as per CLAUDE.md guidelines
  • Uses @std/assert assertions (assertEquals, assertStringIncludes) correctly
  • Follows the existing test patterns from echo_model_test.ts and unified_data_test.ts
  • Good use of withTempDir helper for test isolation

DDD Principles:

  • Tests properly use the domain entities (Definition) and repository interfaces (YamlDefinitionRepository)
  • Tests interact with the system through the CLI commands (application layer) rather than directly manipulating infrastructure
  • The separation of concerns is maintained

Test Coverage:

  • Full E2E flow: create definition → run method → verify output and data
  • Data commands: list, get, versions
  • Partial ID matching for output lookup
  • Field extraction from output data
  • Multiple runs producing separate outputs
  • Data versioning across multiple runs
  • Error handling: non-existent output, model, and field cases

Security:

  • No security concerns - tests create isolated temp directories and clean up properly
  • No hardcoded credentials or sensitive data

Suggestions (Non-Blocking)

  1. Consider extracting shared test types: The inline type annotations like (o: { modelName: string }) are repeated multiple times. While this works fine, extracting a shared interface could improve maintainability:

    interface OutputSearchResult {
      id: string;
      modelName: string;
      methodName: string;
      status: string;
    }
  2. Consider testing edge case with empty repo: The error handling tests are good, but testing behavior with an empty repository (no models at all) could be valuable.

  3. Comment notes about log tests: The comments explaining why log tests are skipped (echo model doesn't produce logs) are helpful for future maintainers.


Verification

The tests follow the established patterns in the codebase and should pass the CI checks (deno check, deno lint, deno fmt, deno run test).

LGTM! This is a valuable addition to the test suite that will help catch regressions in the data output flow.

@github-actions github-actions Bot merged commit 9badaa5 into main Feb 3, 2026
3 checks passed
@github-actions github-actions Bot deleted the chore-e2e-testing branch February 3, 2026 23:59
ianarsenault pushed a commit to ianarsenault/swamp that referenced this pull request Apr 21, 2026
…-club#141) (systeminit#1205)

## Summary

- Fixes [swamp-club#141](https://swamp.club/lab/issues/141): extension
authors whose `_test.ts` imports the model source hit `TS7006` (implicit
`any` on execute parameters) under strict mode.
- Ships a type-only `ModelDefinition<TGlobalArgs>` in
`@systeminit/swamp-testing` that authors opt into via `satisfies
ModelDefinition<typeof Schema>` (or the `defineModel` function-form
alternative). The literal stays identical — only a trailing satisfies
and a type-only import are added.
- `context.globalArgs` narrows to the schema-inferred shape (verified by
probing an undeclared field → TS2339). This is a real type-safety
upgrade, not a TS7006 silencer.
- Default authoring experience is **unchanged** — unannotated execute
params remain the Quick Start. Existing extensions do not need to
migrate. The `: any` workaround keeps working for anyone already using
it.

## What's included

- `packages/testing/model_definition_types.ts` — new type-only file with
`ModelDefinition`, `MethodDefinition`, `CheckDefinition`,
`ResourceOutputSpec`, `FileOutputSpec`, `VersionUpgrade`, `CheckResult`,
and the `defineModel` helper.
- `packages/testing/types.ts` — `MethodContext<TGlobalArgs =
Record<string, unknown>>` (additive, backward compatible).
- `packages/testing/mod.ts` — new "Model authoring" export block.
- `packages/testing/model_definition_types_test.ts` — type-level
regression test asserting both `satisfies` and `defineModel` paths
narrow `globalArgs` under strict+noImplicitAny.
- `src/domain/models/testing_package_compat_test.ts` — extended to guard
drift between testing and canonical `ModelDefinition` /
`MethodDefinition`.
- `.claude/skills/swamp-extension-model/references/typing.md` — new
reference with before/after, "when not to use," and cross-links to the
compat test and issue systeminit#141.
- `.claude/skills/swamp-extension-model/SKILL.md` — Key Rule systeminit#5 points
at typing.md.
- `.claude/skills/swamp-extension-model/references/testing.md` —
cross-link banner at the top.
- `packages/testing/README.md` — new "Model authoring escape hatch"
subsection.
- `packages/testing/deno.json` — bumped 0.2.0 → 0.3.0.

## Author-facing usage

```typescript
import { z } from "npm:zod@4";
import type { ModelDefinition } from "jsr:@systeminit/swamp-testing";

const GlobalArgsSchema = z.object({ region: z.string() });

export const model = {
  type: "@myorg/my-model",
  version: "2026.04.21.1",
  globalArguments: GlobalArgsSchema,
  methods: {
    run: {
      description: "Run the model",
      arguments: z.object({ bucket: z.string() }),
      execute: async (_args, context) => {
        // context.globalArgs narrows to { region: string }
        return { dataHandles: [] };
      },
    },
  },
} satisfies ModelDefinition<typeof GlobalArgsSchema>;
```

## Why not a per-method args-narrowing helper

The plan's initial `defineModel` sketch intended per-method `args`
inference via mapped-intersection generics. Both the F-bounded recursive
constraint and the `Omit`-based override pattern failed contextual
typing during inference — per-method args narrowing for inline literals
requires a builder-style API (as used by tRPC / zod's own `.input()`
chain), which is out of scope here. Authors narrow `args` per method
with `MySchema.parse(args)` at runtime (the canonical Zod way).
`typing.md` documents this.

## Test plan

- [x] `deno check main.ts packages/testing/mod.ts` passes
- [x] `deno run test` — all 4505 existing tests pass; new 4 type-level +
smoke tests pass
- [x] `deno lint` clean
- [x] `deno fmt` applied
- [x] `deno run review-skills` — all skills pass the 90% threshold
- [x] Scratch-repo verification: reproduces TS7006 on the pre-fix shape;
compiles cleanly with `satisfies ModelDefinition<typeof Schema>` and
`defineModel({ ... })`; narrowing probe confirms
`context.globalArgs.region` is typed and
`context.globalArgs.undeclaredField` errors as expected.
- [x] Drift-guard verification: injecting `description: number` into
testing-side `MethodDefinition` immediately failed
`testing_package_compat_test.ts` as designed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant