Skip to content

test(desktop): expand Storybook smoke theme coverage - #4042

Open
Yx01-me wants to merge 2 commits into
apache:mainfrom
Yx01-me:feat/story_book_palette_scheme
Open

test(desktop): expand Storybook smoke theme coverage#4042
Yx01-me wants to merge 2 commits into
apache:mainfrom
Yx01-me:feat/story_book_palette_scheme

Conversation

@Yx01-me

@Yx01-me Yx01-me commented Aug 27, 2026

Copy link
Copy Markdown

Summary

  • Run every Storybook story with the default palette in both light and dark color schemes.
  • Run the Native Conversation reference story across all 11 palettes and both color schemes.
  • Include scheme and palette context in Storybook smoke URLs, diagnostics, and tests without expanding the full catalog across every palette.

Refs #3942

Verification

  • node --test scripts/ci-test-plan.test.mjs scripts/storybook-visual-smoke.test.mjs scripts/ax-tree-audit.test.mjs
  • npm run lint
  • npm run format:check
  • npm run typecheck
  • npm run build
  • npx knip --workspace apps/desktop
  • npx knip --workspace packages/ui
  • npm --workspace @maka/desktop run smoke:storybook — 410 renders across 195 stories

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex assisted with the Storybook theme job matrix implementation, tests, and local verification.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 27, 2026

@Astro-Han Astro-Han 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.

Thanks for broadening the Storybook smoke coverage with a bounded job matrix. Using both color schemes for ordinary stories and the full palette set for one representative product story is a sensible balance.

I found one local-entrypoint regression and left it inline. The contract tests pass, but the hosted test check has not run on this head, and the smoke command currently depends on a Core build that it does not establish itself. I’m holding approval for that fix.

Review analysis was assisted by Codex; Astro-Han reviewed the result and owns this review.

简体中文

谢谢用有界的任务矩阵扩展 Storybook smoke 覆盖。普通 story 覆盖两种色彩模式,同时只让一个代表性的产品 story 覆盖全部 palette,这个取舍合理。

我发现了一处本地入口回归,已经留在行内。Contract tests 已通过,但当前 head 还没有运行 GitHub test check,而且 smoke 命令依赖一个它自己没有保证完成的 Core build,因此这里先不 Approve。

本次审查分析由 Codex 协助;Astro-Han 审阅了结果并对本次 Review 负责。

const staticDir = resolve(process.argv[2] ?? join(repoRoot, 'apps/desktop/storybook-static'));
const storyIndex = await readFile(join(staticDir, 'index.json'), 'utf8').then(JSON.parse);
const jobs = catalogJobs(storyIndex);
const { THEME_PALETTES } = await import('@maka/core/settings');

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.

Thanks for expanding theme coverage without multiplying every story by every palette. [P2] This import adds an undeclared build prerequisite to the smoke entrypoint: @maka/core/settings resolves to dist/settings.js, but npm --workspace @maka/desktop run smoke:storybook does not build @maka/core first. In a clean worktree, building Storybook and then running the smoke command fails with ERR_MODULE_NOT_FOUND; CI hides this because the root build happens earlier. Please make the smoke command establish that prerequisite, or consume the shared palette authority through a path that does not depend on pre-existing Core build output.

简体中文

谢谢在没有让每个 story 都乘上全部 palette 的前提下扩展主题覆盖。[P2] 这个 import 给 smoke 入口增加了一个未声明的构建前置条件:@maka/core/settings 会解析到 dist/settings.js,但 npm --workspace @maka/desktop run smoke:storybook 不会先构建 @maka/core。在干净 worktree 中,先构建 Storybook 再运行 smoke 会得到 ERR_MODULE_NOT_FOUND;CI 因为更早执行了根目录 build,恰好掩盖了这个问题。请让 smoke 命令显式满足该前置条件,或者从不依赖 Core 预构建产物的路径读取共享 palette authority。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll solve it soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants