Skip to content

feat(test): add "test export" / "test import" to round-trip test DEFINITIONS for version control#265

Open
Andy00L wants to merge 4 commits into
TestSprite:mainfrom
Andy00L:feat/test-export-import
Open

feat(test): add "test export" / "test import" to round-trip test DEFINITIONS for version control#265
Andy00L wants to merge 4 commits into
TestSprite:mainfrom
Andy00L:feat/test-export-import

Conversation

@Andy00L

@Andy00L Andy00L commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Round-trip a test DEFINITION (metadata + code with codeVersion provenance) to a versionable JSON file, so definitions can be reviewed, backed up, and migrated.

  • test export <test-id> writes the definition (--out <file> + --force, stdout by default). Frontend plans are write-only on the API, so FE exports carry planUnavailable: true and a stderr note.
  • test import <file> creates or updates from a definition file: a testId in the file selects update (code PUT replays the recorded codeVersion as If-Match, so a drifted server copy fails loudly with the existing 412 contract); no testId creates.
  • 5 unit tests (export shape, FE marker, create vs update paths, If-Match replay); test surface test and help snapshot updated for the two new subcommands.
  • Verified against current main: typecheck, build, full suite (2007 tests), lint and prettier clean.

Closes #125

Discord: interferon0

Summary by CodeRabbit

  • New Features

    • Added test export to generate deterministic, versioned test definition JSON to stdout or --out (supports --dry-run and prevents overwrites unless --force).
    • Added test import to create or update tests from a definition file (validates schema fields, supports --dry-run, and reports created/updated).
    • Frontend exports set planUnavailable and emit a write-only warning.
  • Tests

    • Expanded end-to-end coverage for export/import flows, schema/version validation, BOM-prefixed inputs, overwrite/force behavior, and idempotency-key handling.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dae747a1-fa0a-42e8-ae3a-8956a4159293

📥 Commits

Reviewing files that changed from the base of the PR and between fd5316e and 6c245bd.

📒 Files selected for processing (1)
  • src/commands/test.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/commands/test.test.ts

Walkthrough

Test definition export/import support is added to the test CLI, including deterministic JSON definitions, backend and frontend export behavior, validated create/update imports, concurrency and idempotency handling, file safety, and comprehensive command coverage.

Changes

Test definition round-trip

Layer / File(s) Summary
Definition schema and export/import handlers
src/commands/test.ts
Adds CliTestDefinition, deterministic export with code provenance and frontend markers, plus validated create/update import flows with dry-run and overwrite handling.
Export and import command wiring
src/commands/test.ts
Adds test export <test-id> and test import <file> subcommands.
Export and import behavior coverage
src/commands/test.test.ts
Covers command registration, export/import flows, file errors, schema validation, idempotency keys, concurrency headers, and output protection.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant runExport
  participant runImport
  participant TestAPI
  participant DefinitionFile
  CLI->>runExport: export test definition
  runExport->>TestAPI: fetch metadata and code
  TestAPI-->>runExport: return test data and codeVersion
  runExport->>DefinitionFile: write JSON definition
  CLI->>runImport: import JSON definition
  runImport->>DefinitionFile: read and validate definition
  runImport->>TestAPI: create or update metadata
  runImport->>TestAPI: upload code with If-Match
Loading

Possibly related PRs

Suggested reviewers: ruili-testsprite

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding test export/import for round-trippable definitions.
Linked Issues check ✅ Passed The changes implement the linked #125 objectives: deterministic export/import, schemaVersion, codeVersion provenance, frontend planUnavailable, and update/create flows.
Out of Scope Changes check ✅ Passed The summarized changes stay within the export/import definition feature and related test coverage; no unrelated scope is evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/commands/test.test.ts (1)

3265-3411: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Solid core coverage; a few critical paths are untested.

The suite covers export composition, create/update import, and schemaVersion rejection well. Missing cases that exercise other new branches in runExport/runImport:

  • --out/--force file-write path (including the "already exists" VALIDATION_ERROR when --force is omitted).
  • --dry-run for both commands (banner emission, canned sample shape).
  • The NOT_FOUND code-fetch fallback (a fresh test with no code yet → code omitted from the definition).
  • Required-field validation errors for projectId/type/name (only schemaVersion is currently exercised).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/test.test.ts` around lines 3265 - 3411, Extend the
runExport/runImport test suite to cover the missing branches: verify --out
writes a file and rejects an existing file with VALIDATION_ERROR unless --force
is set; verify --dry-run for both commands emits its banner and returns the
expected canned sample shape; mock a NOT_FOUND code-fetch response and assert
export omits code; and add import cases asserting missing projectId, type, or
name produce field-level VALIDATION_ERROR results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/test.ts`:
- Around line 4252-4265: Align the create path with the validation used by the
update branch: only include the code POST request when def.code is defined and
def.code.body is a string. Update the create-branch condition around the POST
payload while preserving the existing behavior and metadata for valid code
bodies.
- Around line 4243-4280: Update the test import create and update flows around
the POST /tests and PUT /tests/{testId}[/code] calls to accept and reuse a
controllable idempotency key, rather than generating unrelated keys for each
request; when keys are auto-generated, log them to stderr following the existing
create/run behavior, including under JSON output. Expose the key through the
import command’s existing option/configuration path and ensure retries can
replay the same key for both update PUT requests and the create POST.

---

Nitpick comments:
In `@src/commands/test.test.ts`:
- Around line 3265-3411: Extend the runExport/runImport test suite to cover the
missing branches: verify --out writes a file and rejects an existing file with
VALIDATION_ERROR unless --force is set; verify --dry-run for both commands emits
its banner and returns the expected canned sample shape; mock a NOT_FOUND
code-fetch response and assert export omits code; and add import cases asserting
missing projectId, type, or name produce field-level VALIDATION_ERROR results.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c0a9f1ff-1d93-4f37-979c-4281b31c5e5c

📥 Commits

Reviewing files that changed from the base of the PR and between 2708a40 and a843254.

⛔ Files ignored due to path filters (1)
  • test/__snapshots__/help.snapshot.test.ts.snap is excluded by !**/*.snap, !**/*.snap
📒 Files selected for processing (2)
  • src/commands/test.test.ts
  • src/commands/test.ts

Comment thread src/commands/test.ts
Comment thread src/commands/test.ts
@zeshi-du

Copy link
Copy Markdown
Contributor

Maintainer review: request changes — solid concept, five concrete gaps, plus a scope note.

  1. BOM handling (Windows regression): runImport's readFileSync + JSON.parse doesn't go through stripBom() — every other JSON file-read in test.ts does, precisely because PowerShell 5.1's Set-Content -Encoding utf8 writes a BOM (see the helper's own doc comment). As written, test import fails on exactly the platform CI tests for. Add stripBom() + a BOM-prefixed test file case.
  2. Idempotency parity: the import create path mints 'idempotency-key': cli-import-create-${randomUUID()} internally, with no --idempotency-key flag and no stderr echo. Every other mutating command in this file (create, create-batch, update, delete, code put, plan put) exposes the flag + auto-mint + stderr echo, for the reason documented at the test create site: a transport-level retry after an ambiguous failure must be able to reuse the same key, or it duplicates the test. runImport needs the same triad on both the create POST and the code PUT.
  3. codeVersion: null on the update path silently omits If-Match → unconditional overwrite with no warning. code put handles this exact case with If-Match: '*' plus a stderr notice ("auto-fetched codeVersion=null on legacy row..."). Match that, and add a test — this branch currently has zero coverage.
  4. --out/--force file-write path is untested: all five new tests exercise stdout export only; the overwrite guard and the actual file write are the safety-critical surface here.
  5. Error conflation: a typo'd path (ENOENT) and malformed JSON produce the same "not readable valid JSON" message. Split them — "file not found: " vs the JSON error.

Scope note: issue #125 was triaged deferred on 07-03 (schema-freeze vs in-flight server-side test-entity work) and that hold hasn't formally lifted — we're re-evaluating it now that the wire contract is stability-committed, and will record the outcome on #125. Fixing the five items above is worthwhile regardless of timing; landing waits on that call.

@Andy00L

Andy00L commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

All five points addressed:

  1. runImport now reads through stripBom() + a BOM-prefixed test case.
  2. Idempotency triad on both mutations: --idempotency-key flag on test import, auto-mint cli-import-<uuid>, stderr echo (create-site condition); per-mutation keys derive deterministically (:meta/:code/:create) so a replayed key reuses the same wire keys.
  3. codeVersion: null now sends If-Match: * with a stderr notice (mirrors code put's legacy-row handling) + dedicated test.
  4. --out/--force file-write path covered: write, overwrite-guard rejection, forced overwrite.
  5. ENOENT and malformed JSON are split into file not found: <path> vs is not valid JSON: <path>.
    Full suite green (2012 tests). Understood on the scope hold — ready whenever the [Hackathon] Add "test export" / "test import" to round-trip test DEFINITIONS for version control #125 re-evaluation lands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/test.test.ts`:
- Around line 3415-3424: Replace the literal BOM character at the start of the
template literal passed to writeFileSync with the escaped \uFEFF sequence,
preserving the generated file’s BOM while allowing lint to pass.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b3435cc0-cbbc-485e-ac76-75fdfddbdf58

📥 Commits

Reviewing files that changed from the base of the PR and between a843254 and 5f98531.

⛔ Files ignored due to path filters (1)
  • test/__snapshots__/help.snapshot.test.ts.snap is excluded by !**/*.snap, !**/*.snap
📒 Files selected for processing (2)
  • src/commands/test.test.ts
  • src/commands/test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/commands/test.ts

Comment thread src/commands/test.test.ts
@Andy00L

Andy00L commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Lint red fixed: the BOM fixture sat in a template literal, which no-irregular-whitespace rejects (plain strings are exempt, which is why the existing BOM fixtures pass). Moved to a concatenated string; eslint and the suite are green.

@Andy00L

Andy00L commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Adopted the suggested \uFEFF escape form for the BOM fixture (the interim concatenated-string fix already unblocked lint; this matches the proposed diff exactly).

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.

[Hackathon] Add "test export" / "test import" to round-trip test DEFINITIONS for version control

2 participants