Skip to content

Unskip the listing-create suite, updating tests to current command behavior - #681

Merged
richardhjtan merged 4 commits into
mainfrom
fix/unskip-listing-create
Aug 3, 2026
Merged

Unskip the listing-create suite, updating tests to current command behavior#681
richardhjtan merged 4 commits into
mainfrom
fix/unskip-listing-create

Conversation

@richardhjtan

Copy link
Copy Markdown
Collaborator

Turns the listing-create live suite back on. The suite was skipped wholesale by the host→catalog migration and its tests had drifted behind three deliberate behavior changes — they were asserting stale contracts, not catching bugs:

  1. Auto-patch is background work now. The command returns before autoPatchName/Summary/autoLink/linkSpecs run; they land via the backgroundWork promise on the result (the create-listing-modal awaits it the same way). Tests now await it before asserting those fields.
  2. Only the listed export gets a spec. linkSpecs deliberately specs just the listed export of the main module, so a module defining both Author and AuthorCompany yields one spec.
  3. SearchAndChoose speaks numbered options. Candidates go to the LLM as an id-less numbered list (anti-hallucination) and the reply is option numbers. The mock now parses the wanted entry's number out of the prompt, and link assertions compare card names instead of realm-specific ids — the candidate pool spans every readable realm (mock + real catalog), so ids aren't deterministic.

Two tests remain skipped with reasons in the file: they're written against the pre-storefront isolated-view DOM (data-test-catalog-listing-embedded-*) and the retired open-on-stack flow — the create-listing-modal now navigates to code mode with an isolated preview after the command runs. They need a rewrite against that flow (same bucket as the browse-suite DOM rewrite).

Test plan

  • listing create suite locally: 2 pass / 2 skip / 0 fail
  • recognised imports test passes unchanged (guards spec-creation realm boundaries)

Part of the Eliminate Skipped Tests in Catalog CI project (CS-11462).

🤖 Generated with Claude Code

…havior

The suite predated three deliberate behavior changes, so the tests
asserted stale contracts rather than catching bugs:

- Auto-patch work (name, summary, links, specs) moved off the command's
  critical path onto the result's backgroundWork promise; tests now await
  it before asserting those fields.
- Only the listed export of the main module gets a spec, so a module
  defining both Author and AuthorCompany yields one spec, not two.
- SearchAndChoose presents candidates as an id-less numbered list and
  expects option numbers back; the LLM mock now answers with the wanted
  entry's number, and link assertions compare card names instead of
  realm-specific ids (the candidate pool spans every readable realm).

Two tests stay skipped with reasons in place: they're written against the
pre-storefront isolated-view DOM and the retired open-on-stack flow (the
create-listing-modal now navigates to code mode with an isolated preview).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 05:50
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

Staging Submissions Preview

This PR's content is pushed to the staging submissions realm: https://realms-staging.stack.cards/submissions/

Changed folders:

  • tests/

Updated at 2026-08-03 04:14:09 UTC for commit 65933eb. Shared realm: only this PR's changed files are pushed; files touched by multiple PRs reflect whichever pushed last, and deleted files are not removed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Re-enables the listing create live acceptance suite and updates the tests to match current ListingCreateCommand behavior (notably: async background auto-patch/link work, updated spec-linking semantics, and SearchAndChoose’s numbered-option prompting).

Changes:

  • Unskips the listing create suite and converts key cases from skip to test.
  • Updates the proxy/LLM mock to handle SearchAndChoose’s numbered candidate list and asserts linked entities by stable names instead of realm-specific ids.
  • Awaits result.backgroundWork before asserting auto-patched/link-populated fields and updates spec assertions to expect only the listed export to be spec’d.
Suppressed comments (1)

tests/live/catalog-app/listing-create.test.gts:345

  • This test is now un-skipped, but it never awaits the command’s backgroundWork. Since linkSpecs runs in the background and backgroundWork uses allSettled (doesn’t throw), the later listing.specs.every(...) assertion can pass vacuously when specs is still empty, reducing the test’s ability to catch regressions. Await backgroundWork and assert that specs were actually generated.
          test('listing will only create specs with recognised imports from realms it can read from', async function (assert) {
            const cardId = mockCatalogURL + 'UnrecognisedImports/example';
            const commandService = getService('tool-service');
            const command = new ListingCreateCommand(
              commandService.commandContext,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/live/catalog-app/listing-create.test.gts Outdated
richardhjtan and others added 2 commits July 31, 2026 15:51
Always answer with a JSON selection (empty for candidate types the mock
doesn't know) and match the candidate-type marker case-insensitively.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The open-on-stack test asserted navigation the command no longer does —
the create-listing-modal owns that flow (code mode with isolated preview)
— so it's gone rather than skipped.

In its place, cover the supportingCards contract: open cards that aren't
instances of the listed type are linked as supportingCards, explicit
supportingCardIds are honored verbatim, and a card classified as an
example is never duplicated into supportingCards. Assertions read the
persisted document's relationships — that's what the command writes; the
in-memory instance's linksToMany entries resolve lazily.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@richardhjtan

Copy link
Copy Markdown
Collaborator Author

[Claude Code 🤖] Two updates in d423931, following review discussion:

  • Deleted the open-on-stack test instead of leaving it skipped — it asserted navigation the command no longer performs (the create-listing-modal owns the post-create flow now, navigating to code mode with an isolated preview), so there's nothing left for it to guard at the command level.
  • Added supportingCards coverage, which the suite previously had none of: open cards that aren't instances of the listed type get linked as supportingCards, explicit supportingCardIds are honored verbatim, and a card classified as an example is never duplicated into supportingCards. The assertions read the persisted document's relationships rather than the in-memory instance — the doc is what the command writes, and linksToMany entries on the live instance resolve lazily (the raw doc showed supportingCards.1 present while the instance still reported it as unresolved).

Suite now: 3 pass / 1 skip (the app-listing test, pending the storefront-DOM rewrite) / 0 fail.

The create command can no longer produce an AppListing — ListingType is
card/skill/theme/field/component — so the test exercised a retired
creation path (on top of asserting the pre-storefront DOM). The
AppListing card class itself remains for existing instances.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@richardhjtan
richardhjtan requested a review from a team August 3, 2026 04:29
@richardhjtan
richardhjtan merged commit ef3015f into main Aug 3, 2026
3 checks passed
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