fix(tests): update scanUsedIcons tests to include required radio icons#6894
fix(tests): update scanUsedIcons tests to include required radio icons#6894Soulter merged 1 commit intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:S
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在修复 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| const icons = [...scanUsedIcons(collectFiles(tmp, ['.vue']))]; | ||
| assert.equal(icons.filter(icon => icon === 'mdi-radiobox-marked').length, 1); | ||
| assert.ok(icons.includes('mdi-radiobox-blank')); | ||
| assert.ok(icons.includes('mdi-home')); |
There was a problem hiding this comment.
这个测试用例的断言是正确的，但可以通过直接操作 scanUsedIcons 返回的 Set 对象来简化和增强。
将 Set 转换为数组并使用 filter 和 includes 来检查成员资格和唯一性，在语法上有些繁琐，而且效率低于直接使用 Set 的 has 方法和 size 属性。
建议直接在 Set 对象上进行断言，这样代码更简洁，也更能体现测试的意图——验证集合的内容和大小。
| const icons = [...scanUsedIcons(collectFiles(tmp, ['.vue']))]; | |
| assert.equal(icons.filter(icon => icon === 'mdi-radiobox-marked').length, 1); | |
| assert.ok(icons.includes('mdi-radiobox-blank')); | |
| assert.ok(icons.includes('mdi-home')); | |
| const icons = scanUsedIcons(collectFiles(tmp, ['.vue'])); | |
| assert.ok(icons.has('mdi-radiobox-marked')); | |
| assert.ok(icons.has('mdi-radiobox-blank')); | |
| assert.ok(icons.has('mdi-home')); | |
| assert.equal(icons.size, 3); |
dosubot
bot
added
the
area:webui
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the deduplication test you convert the Set to an array and use
filter/includes; consider asserting via Set APIs andicons.sizedirectly to better express the deduplication contract and avoid unnecessary transformations. - The three tests that create temporary directories each duplicate the
makeTmpDir/writeFileSync/rmSyncpattern; consider extracting a small helper or usingtry/finallyaroundrmSyncto reduce repetition and make cleanup more robust if an assertion fails.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the deduplication test you convert the Set to an array and use `filter`/`includes`; consider asserting via Set APIs and `icons.size` directly to better express the deduplication contract and avoid unnecessary transformations.
- The three tests that create temporary directories each duplicate the `makeTmpDir`/`writeFileSync`/`rmSync` pattern; consider extracting a small helper or using `try/finally` around `rmSync` to reduce repetition and make cleanup more robust if an assertion fails.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
3 participants
Motivation / 动机
修复
dashboard/scripts/subset-mdi-font.mjs在图标子集化后遗漏 Vuetifyv-radio间接依赖图标的问题。在 fix(ui): include vuetify radiobox icons #6892 中，已通过
REQUIRED_ICONS将mdi-radiobox-blank和mdi-radiobox-marked加入usedIcons的初始化，解决了 Radio 按钮无法正确渲染的问题；但这也改变了scanUsedIcons()的行为，使其在源码中没有任何mdi-*图标时也会返回这两个必需图标，导致原有单元测试中关于“返回空集”或“集合大小”的断言不再准确。本 PR 仅更新对应单元测试，使测试行为与当前实现保持一致，确保测试套件能够准确反映
REQUIRED_ICONS的新逻辑并稳定通过。Modifications / 改动点
更新
dashboard/tests/subsetMdiFont.test.mjs调整
scanUsedIcons相关测试断言，使其适配REQUIRED_ICONS默认注入的行为将“无图标时返回空集”的旧测试更新为“至少包含两个 Radio 必需图标”
更新已有集合大小断言，计入
mdi-radiobox-blank与mdi-radiobox-marked新增去重测试，验证当源码中已包含
mdi-radiobox-marked时不会重复加入This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
已执行以下测试命令：
测试结果：
本次修改后，subsetMdiFont.test.mjs 全部测试通过，测试行为已与 REQUIRED_ICONS 新逻辑保持一致。
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
/ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到 requirements.txt 和 pyproject.toml 文件相应位置。
😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Align scanUsedIcons unit tests with the updated behavior that always includes required Vuetify radio icons.
Tests: