Conversation
|
WalkthroughAdded an extensive test suite for the theme system in Changes
Sequence Diagram(s)(omitted — changes are tests only; no control-flow feature to diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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. Comment |
|
Triggered from #274 by @black7375. Checking if we can fast forward Target branch ( commit 4a185b470a8d24ecc6badcb46ea4c0408e486a07 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Sep 27 00:00:00 2025 +0900
Feat: `theme` - Support `alias` method #270Pull request ( commit 7a04469e1bfb5da02c0af0195e0869bcae93691f (pull_request/theme-test-code)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Tue Nov 4 02:08:14 2025 +0900
Test: add `theme()` test casesIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/css/src/theme/index.ts (2)
1381-1385: Verify the regex pattern matches actual identifier format.The regex
/^[a-zA-Z_][a-zA-Z0-9_]*$/might not match the actual format generated bygenerateIdentifier(). Based on theidentifierNameutility (lines 4-10 inutils.ts), identifiers typically include hash suffixes in the formatname__hash. Consider using a pattern that accounts for the hash suffix, or use theidentifierName()utility without arguments.Apply this diff to align with the actual identifier format:
- expect(className).toMatch(/^[a-zA-Z_][a-zA-Z0-9_]*$/); + expect(className).toMatch(identifierName());
1887-1903: Consider whether this integration test adds value.This test verifies that
theme()integrates correctly withglobalTheme(). However, sincetheme()internally callsglobalTheme()(line 78), all previous tests are already exercising this integration. This test might be slightly redundant, though it can be kept for explicitness if preferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/css/src/theme/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/css/src/theme/index.ts (2)
packages/css/src/index.ts (1)
theme(42-42)packages/css/src/utils.ts (1)
identifierName(5-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (1)
packages/css/src/theme/index.ts (1)
1372-1910: Excellent test coverage for the theme system!The test suite is comprehensive and well-structured, covering:
- ClassName generation (with/without debugId)
- Token types (primitive, array, nested, TokenDefinition, TokenCompositeValue, TokenUnitValue)
- Semantic token features (fallbackVar, alias, raw)
- Edge cases (empty theme, deeply nested structures, mixed types)
- Complex scenarios (semantic tokens referencing other semantic tokens)
The tests are clear, maintainable, and use proper utilities for normalization and validation.
7a04469 to
4ef0375
Compare
|
Triggered from #274 by @black7375. Checking if we can fast forward Target branch ( commit 4a185b470a8d24ecc6badcb46ea4c0408e486a07 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Sep 27 00:00:00 2025 +0900
Feat: `theme` - Support `alias` method #270Pull request ( commit 4ef03753991571c260a79307cde319486b40d6b4 (pull_request/theme-test-code)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Fri Oct 10 00:00:00 2025 +0900
Test: add `theme()` test casesIt is possible to fast forward |
|
/fast-forward |
|
Triggered from #274 (comment) by @black7375. Trying to fast forward Target branch ( commit 4a185b470a8d24ecc6badcb46ea4c0408e486a07 (HEAD -> main, origin/main)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Sep 27 00:00:00 2025 +0900
Feat: `theme` - Support `alias` method #270Pull request ( commit 4ef03753991571c260a79307cde319486b40d6b4 (pull_request/theme-test-code)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Fri Oct 10 00:00:00 2025 +0900
Test: add `theme()` test casesFast forwarding $ git push origin 4ef03753991571c260a79307cde319486b40d6b4:main
To https://github.com/mincho-js/mincho.git
4a185b4..4ef0375 4ef03753991571c260a79307cde319486b40d6b4 -> main |
Description
Add more test cases.
Related Issue
Summary by CodeRabbit
Additional context
Checklist