Skip to content

chore(skill): direct primitives to tests/display/ and reserve tests/utils/ for helpers#1049

Merged
DreaminDani merged 1 commit into
mainfrom
chore/skill-tests-folder-guidance
May 27, 2026
Merged

chore(skill): direct primitives to tests/display/ and reserve tests/utils/ for helpers#1049
DreaminDani merged 1 commit into
mainfrom
chore/skill-tests-folder-guidance

Conversation

@DreaminDani
Copy link
Copy Markdown
Contributor

@DreaminDani DreaminDani commented May 27, 2026

Summary

Codify in the CSS Modules migration skill where component specs belong, so future migration PRs land in the right test folder on the first try.

Copilot raised the same observation on the Spacer (#1045) and Separator (#1046) review: those specs were placed under tests/utils/, which is reserved for shared helpers like getStoryUrl (tests/utils/index.ts). Component specs belong in component-family folders.

Changes

  • Add a "Pick <area> carefully" subsection in the skill explaining:
    • tests/utils/ is reserved for shared helpers — never put component specs there.
    • Existing component-family folders (tests/buttons/, tests/cards/) take precedence.
    • Primitives (Spacer, Separator, Text, Title, Label, GenericLabel, Icon, etc.) go in tests/display/.
    • New folders are named for the family (tests/forms/, tests/overlays/), not the individual component.
  • Note that sibling specs import the helper as from '../utils'.

Follow-ups (separate PRs)


Note

Low Risk
Documentation-only change to an internal migration skill; no runtime or test code modified.

Overview
Updates the component CSS Modules migration skill so commit 1’s Playwright spec path is chosen deliberately instead of defaulting to tests/utils/.

The skill now spells out that tests/utils/ is helpers-only (e.g. getStoryUrl), that existing families like tests/buttons/ and tests/cards/ should be reused when they fit, that display primitives (Spacer, Separator, Text, Icon, etc.) belong in tests/display/, and that new folders should be named for a component family rather than a single component. It also documents importing shared helpers as from '../utils' from a sibling test folder.

Reviewed by Cursor Bugbot for commit e946756. Bugbot is set up for automated code reviews on this repo. Configure here.

…tils/ for helpers

Address feedback Copilot raised on the Spacer and Separator migration PRs:
component specs were landing in tests/utils/ next to the shared getStoryUrl
helper, which conflates test fixtures with test code.

Codify in the skill:
- tests/utils/ is reserved for shared helpers (getStoryUrl, etc.).
- Primitives (Spacer, Separator, Text, Title, Label, GenericLabel, Icon,
  ...) belong in tests/display/.
- New folders only for new component families, named for the family.
- Sibling specs import getStoryUrl as `from '../utils'`.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@DreaminDani DreaminDani self-assigned this May 27, 2026
@DreaminDani DreaminDani requested a review from Copilot May 27, 2026 17:33
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 27, 2026

⚠️ No Changeset found

Latest commit: e946756

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@workflow-authentication-public
Copy link
Copy Markdown
Contributor

Storybook Preview Deployed

✅ Preview URL: https://click-gro4cpzdd-clickhouse.vercel.app

Built from commit: 235de18cc2c2a1d8e1f09f38c7d33f3792cc074b

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Documentation-only change to the CSS Modules migration skill, clarifying where new Playwright specs should be placed so future migration PRs don't default to tests/utils/.

Changes:

  • Adds explicit guidance that tests/utils/ is reserved for shared helpers only.
  • Directs display primitives (Spacer, Separator, Text, etc.) to tests/display/ and documents naming rules for new family folders.
  • Notes the sibling import path from '../utils' for getStoryUrl.

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

DreaminDani added a commit that referenced this pull request May 27, 2026
Address Copilot's standing review feedback on the migration PRs:
tests/utils/ is reserved for shared helpers like getStoryUrl;
component specs belong in component-family folders. Migration skill
updated in PR #1049 to codify this for future migrations.

Visual regression remains byte-for-byte: 8/8 snapshots match without
regeneration.
DreaminDani added a commit that referenced this pull request May 27, 2026
Address Copilot's standing review feedback on the migration PRs:
tests/utils/ is reserved for shared helpers like getStoryUrl;
component specs belong in component-family folders. Migration skill
updated in PR #1049 to codify this for future migrations.

Visual regression remains byte-for-byte: 37/37 snapshots match without
regeneration.
@DreaminDani DreaminDani merged commit ee03d67 into main May 27, 2026
14 checks passed
@DreaminDani DreaminDani deleted the chore/skill-tests-folder-guidance branch May 27, 2026 17:59
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