Skip to content

Add config pipeline snapshot tests#6947

Open
ryancbahan wants to merge 3 commits intomainfrom
rcb/config-model-snapshot-tests
Open

Add config pipeline snapshot tests#6947
ryancbahan wants to merge 3 commits intomainfrom
rcb/config-model-snapshot-tests

Conversation

@ryancbahan
Copy link
Contributor

@ryancbahan ryancbahan commented Mar 6, 2026

WHAT is this pull request doing?

Adds snapshot tests for writeAppConfigurationFile and the full config round-trip:

  • Round-trip fidelity — write → read → write produces identical output (documents the existing webhook compliance reordering asymmetry, which is fixed in a later PR in this stack)
  • Round-trip stabilization — verifies idempotency after the first cycle
  • Mixed webhook topics — regular topics + compliance topics + include_fields
  • Relative URI round-trip — verifies /webhooks paths survive the cycle unchanged
  • Webhook edge cases — same URI with different filter values, same URI with different include_fields, mixed topics + compliance_topics on a single subscription

No production code changes. Purely additive test file.

WHY are these changes introduced?

The config pipeline (write → read → write round-trip) has no CLI command that exercises it end-to-end — app deploy reads config but doesn't write it back, and app config link writes but doesn't round-trip. This means there's no natural place to add e2e tests for config read/write regressions. Unit-level snapshot tests are the right tool here because:

  1. No CLI command does a clean round-trip. An e2e test would need to orchestrate multiple commands and diff file output, adding complexity without additional signal over calling the pipeline functions directly. (Maybe we get here later)
  2. Snapshot diffs are precise. When a change alters TOML output (field ordering, whitespace, section headers), the snapshot diff shows exactly what changed, making review trivial. An e2e test would need custom diffing logic to achieve the same clarity.
  3. Fast feedback loop. These run in ~30ms total vs minutes for e2e. During refactoring (path extraction, transform extraction, webhook merging changes), fast iteration matters.
  4. They test the actual pipeline functions (writeAppConfigurationFile + parseConfigurationFile) — the same code paths that every CLI command uses. No mocking, no indirection.

How to test your changes?

npx vitest run packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@ryancbahan ryancbahan changed the title Add config pipeline snapshot tests 4/N: Add config pipeline snapshot tests (safety net) Mar 6, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 79.23% 14599/18426
🟡 Branches 73.49% 7249/9864
🟡 Functions 79.37% 3717/4683
🟡 Lines 79.56% 13795/17339

Test suite run success

3830 tests passing in 1466 suites.

Report generated by 🧪jest coverage report action from a29ba87

@github-actions
Copy link
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/public/node/toml/codec.d.ts
import { JsonMap } from '../../../private/common/json.js';
export type JsonMapType = JsonMap;
/**
 * Given a TOML string, it returns a JSON object.
 *
 * @param input - TOML string.
 * @returns JSON object.
 */
export declare function decodeToml(input: string): JsonMapType;
/**
 * Given a JSON object, it returns a TOML string.
 *
 * @param content - JSON object.
 * @returns TOML string.
 */
export declare function encodeToml(content: JsonMap | object): string;
packages/cli-kit/dist/public/node/toml/index.d.ts
export type { JsonMapType } from './codec.js';
packages/cli-kit/dist/public/node/toml/toml-file.d.ts
import { JsonMapType } from './codec.js';
/**
 * Thrown when a TOML file cannot be parsed. Includes the file path for context.
 */
export declare class TomlParseError extends Error {
    readonly path: string;
    constructor(path: string, cause: Error);
}
/**
 * General-purpose TOML file abstraction.
 *
 * Provides a unified interface for reading, patching, removing keys from, and replacing
 * the content of TOML files on disk.
 *
 * - `read` populates content from disk
 * - `patch` does surgical WASM-based edits (preserves comments and formatting)
 * - `remove` deletes a key by dotted path (preserves comments and formatting)
 * - `replace` does a full re-serialization (comments and formatting are NOT preserved).
 * - `transformRaw` applies a function to the raw TOML string on disk.
 */
export declare class TomlFile {
    /**
     * Read and parse a TOML file from disk. Throws if the file doesn't exist or contains invalid TOML.
     * Parse errors are wrapped in {@link TomlParseError} with the file path for context.
     *
     * @param path - Absolute path to the TOML file.
     * @returns A TomlFile instance with parsed content.
     */
    static read(path: string): Promise<TomlFile>;
    readonly path: string;
    content: JsonMapType;
    constructor(path: string, content: JsonMapType);
    /**
     * Surgically patch values in the TOML file, preserving comments and formatting.
     *
     * Accepts a nested object whose leaf values are set in the TOML. Intermediate tables are
     * created automatically. Setting a leaf to `undefined` removes it (use `remove()` for a
     * clearer API when deleting keys).
     *
     * @example
     * ```ts
     * await file.patch({build: {dev_store_url: 'my-store.myshopify.com'}})
     * await file.patch({application_url: 'https://example.com', auth: {redirect_urls: ['...']}})
     * ```
     */
    patch(changes: {
        [key: string]: unknown;
    }): Promise<void>;
    /**
     * Remove a key from the TOML file by dotted path, preserving comments and formatting.
     *
     * @param keyPath - Dotted key path to remove (e.g. 'build.include_config_on_deploy').
     * @example
     * ```ts
     * await file.remove('build.include_config_on_deploy')
     * ```
     */
    remove(keyPath: string): Promise<void>;
    /**
     * Replace the entire file content. The file is fully re-serialized — comments and formatting
     * are NOT preserved.
     *
     * @param content - The new content to write.
     * @example
     * ```ts
     * await file.replace({client_id: 'abc', name: 'My App'})
     * ```
     */
    replace(content: JsonMapType): Promise<void>;
    /**
     * Transform the raw TOML string on disk. Reads the file, applies the transform function
     * to the raw text, writes back, and re-parses to keep `content` in sync.
     *
     * Use this for text-level operations that can't be expressed as structured edits —
     * e.g. Injecting comments or positional insertion of keys in arrays-of-tables.
     * Subsequent `patch()` calls will preserve any comments added this way.
     *
     * @param transform - A function that receives the raw TOML string and returns the modified string.
     * @example
     * ```ts
     * await file.transformRaw((raw) => `# Header comment\n${raw}`)
     * ```
     */
    transformRaw(transform: (raw: string) => string): Promise<void>;
    private decode;
}

Existing type declarations

We found no diffs with existing type declarations

@ryancbahan ryancbahan changed the base branch from rcb/toml-rename-builders to graphite-base/6947 March 11, 2026 15:23
ryancbahan and others added 3 commits March 11, 2026 16:10
Safety net for config model refactoring. These tests lock the current
behavior of writeAppConfigurationFile and the read→write round-trip:

- Full config TOML output snapshot
- Webhook round-trip reordering documented (known asymmetry: compliance
  subscriptions sort first after mergeAllWebhooks fires during parse)
- Round-trip stabilization test (idempotent after first reorder)
- Mixed webhook topics + compliance topics snapshot
- Relative URI round-trip
- Minimal config snapshot

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan force-pushed the rcb/config-model-snapshot-tests branch from 8ae41eb to a29ba87 Compare March 11, 2026 22:13
@ryancbahan ryancbahan changed the base branch from graphite-base/6947 to main March 11, 2026 22:13
@ryancbahan ryancbahan marked this pull request as ready for review March 11, 2026 22:26
@ryancbahan ryancbahan requested a review from a team as a code owner March 11, 2026 22:26
@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

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.

2 participants