-
Notifications
You must be signed in to change notification settings - Fork 0
chore(daily_read): 完善每日阅读功能的测试覆盖和文档规范 #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- 修复DailyReadScreen中loading条件判断,从`lastValue == null`改为`lastValue != null && lastValue.isEmpty` - 新增每日阅读页面widget测试,包含书签列表显示和loading状态测试 - 测试使用真实Command实例而非复杂Mock,确保与生产代码配置一致 - 添加Command执行状态验证和异步操作清理 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
feat: 实现每日阅读功能的UI测试和ViewModel测试 test: 添加书签操作和状态管理的测试用例
Walkthrough本次变更主要新增两份关于“每日阅读”功能的设计/交互文档,补充了对应的 ViewModel 与 Widget 测试及其 Mockito 生成的桩类文件,并在 daily_read_screen.dart 中调整了 CommandBuilder 的加载判定条件。另对 CLAUDE.md 增加了一行路径引用。 Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant W as DailyReadScreen
participant C as CommandBuilder(load)
participant VM as DailyReadViewModel
U->>W: 打开页面
W->>C: 订阅 load 命令
C->>C: 判断 lastValue(非空且为空列表时显示加载)
C->>VM: 执行 load
VM-->>C: 返回书签列表
C-->>W: 传递结果并触发重建
W-->>U: 渲染列表/庆祝/错误界面
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @shadowfish07, 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!
此拉取请求主要围绕“每日阅读”功能进行,旨在提升其稳定性、可维护性及文档完备性。它修复了加载状态显示的一个关键bug,并大幅增加了UI组件和业务逻辑的测试覆盖,同时引入了详细的功能规格说明和交互流程图,为未来的开发和维护提供了坚实的基础。
Highlights
- 修复加载状态显示bug: 修正了“每日阅读”页面加载状态的显示逻辑,确保用户体验的准确性。
- 增强ViewModel测试覆盖: 为“每日阅读”功能的ViewModel层增加了全面的单元测试,确保核心业务逻辑的健壮性。
- 新增Widget测试用例: 引入了针对“每日阅读”UI组件的Widget测试,覆盖了多种UI状态和用户交互场景。
- 新增功能规格说明文档: 提供了详细的
daily_read_feature_specification.md,全面阐述了“每日阅读”功能的核心定位、业务需求、系统架构、核心逻辑、用户交互、状态管理、UI设计、性能优化、错误处理、用户体验和测试策略。 - 新增交互流程图文档: 增加了
daily_read_interaction_flows.md,通过Mermaid图表清晰地展示了数据加载、UI状态切换、Command执行、用户路径及生命周期管理等交互流程。 - 新增测试辅助文件: 包含了为测试生成的mock文件,以支持ViewModel和Widget测试的隔离性。
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
本次 Pull Request 提交的代码质量很高。您不仅修复了每日阅读页面加载状态的显示逻辑 bug,还极大地提升了该功能的健壮性。
新增的 ViewModel 和 Widget 测试用例覆盖了各种核心场景,确保了业务逻辑和 UI 状态的正确性。
此外,您撰写的两份新文档——功能规格说明和交互流程图——非常详尽和规范,对项目的长期维护非常有价值。
我只在功能规格文档中发现了一个关于 dispose 方法示例代码的小问题,并提出了修改建议。
总体而言,这是一次出色的贡献,感谢您的努力!
|
There is a problem with the Gemini CLI PR review. Please check the action logs for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
=========================================
+ Coverage 9.15% 15.92% +6.76%
=========================================
Files 66 66
Lines 3986 3987 +1
=========================================
+ Hits 365 635 +270
+ Misses 3621 3352 -269
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
lib/ui/daily_read/widgets/daily_read_screen.dart (2)
52-89: 错误监听器可能造成内存泄漏在
didChangeDependencies中创建的错误监听器没有被正确清理。每次依赖变化时都会创建新的监听器,但旧的监听器不会被取消订阅。建议将监听器移到
initState中,并在dispose中取消订阅:+ late StreamSubscription _loadErrorSubscription; + late StreamSubscription _toggleArchivedErrorSubscription; + late StreamSubscription _toggleMarkedErrorSubscription; @override void initState() { super.initState(); // 初始化礼花控制器 _confettiController = ConfettiController( duration: const Duration(seconds: 3), ); // 设置书签归档回调 widget.viewModel.setOnBookmarkArchivedCallback(_onBookmarkArchived); + + // 设置错误监听 + _loadErrorSubscription = widget.viewModel.load.errors + .where((x) => x != null) + .listen((error) { + if (mounted) { + appLogger.e('加载书签失败', error: error); + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar(content: Text('加载书签失败')), + ); + } + }); + // ... 其他错误监听器 } @override void dispose() { + _loadErrorSubscription.cancel(); + _toggleArchivedErrorSubscription.cancel(); + _toggleMarkedErrorSubscription.cancel(); // 释放动画控制器 _confettiController.dispose(); // 清除回调 widget.viewModel.setOnBookmarkArchivedCallback(null); super.dispose(); } - - @override - void didChangeDependencies() { - super.didChangeDependencies(); - // 移除所有错误监听器代码 - }
196-208: 标签更新错误处理不一致
updateBookmarkLabels使用了catchError来处理错误,而其他操作(如toggleBookmarkMarked)通过 Command 的错误流处理。这种不一致的错误处理方式可能导致维护困难。建议将
updateBookmarkLabels也改为 Command 模式,保持错误处理的一致性:- onUpdateLabels: (bookmark, labels) { - widget.viewModel - .updateBookmarkLabels(bookmark, labels) - .catchError((error) { - if (context.mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text('更新标签失败: $error'), - duration: const Duration(seconds: 3), - ), - ); - } - }); - }, + onUpdateLabels: (bookmark, labels) => + widget.viewModel.updateBookmarkLabels.execute( + UpdateLabelsParam(bookmark: bookmark, labels: labels), + ),同时需要在 ViewModel 中添加相应的 Command 并在错误监听器中统一处理。
🧹 Nitpick comments (9)
CLAUDE.md (1)
1-1: 路径引用格式不标准,建议改为可点击的 Markdown 链接或明确的相对路径当前仅以“@.trae/...”形式罗列,不是标准 Markdown 链接,工具或阅读器无法直接跳转。建议改为相对路径引用或显式链接,示例如下。
-@.trae/rules/project_rules.md +/.trae/rules/project_rules.md + +或使用可点击的 Markdown 链接: +- [项目规则](/.trae/rules/project_rules.md)docs/daily_read_feature_specification.md (3)
35-56: 为代码块补充语言标识以通过 markdownlint(MD040)该处为 ASCII 图示,建议标记为 text,避免 lint 报警并提升渲染一致性。类似的“无语言”代码块在文档中多处出现(例如 Line 89、108、129、152、161、169、178、270、390 等),可一并修正。
-``` +```text ┌─────────────────────────────────────┐ │ UI层 (View) │ ... └─────────────────────────────────────┘ -``` +```
189-193: “加载状态”的触发条件表述与实现细节不完全一致,建议精确化文档写“Command正在执行且结果为空”。根据本 PR 对 daily_read_screen.dart 的改动(whileExecuting 条件从 lastValue == null 调整为 lastValue != null && lastValue.isEmpty),建议将条件精确描述为“Command 执行中,且 lastValue 非空且为空集合”,避免与旧行为混淆。
-- **触发条件**: Command正在执行且结果为空 +- **触发条件**: Command 正在执行,且 lastValue != null 且 lastValue.isEmpty
126-126: 文档内联代码位置标注易随代码漂移失真,建议去除具体行号使用 “daily_read_screen.dart:106-223” 这种行号锚定随代码演进容易失效,建议改为段落标题、方法名或“就近注释”的链接方式,或保留文件名+方法名不写行号。
-**代码实现**: `DailyReadScreen.render()` (`daily_read_screen.dart:106-223`) +**代码实现**: `DailyReadScreen.render()`(详见 daily_read_screen.dart)docs/daily_read_interaction_flows.md (2)
71-83: 为纯文本流程代码块补全语言标识(text)以满足 MD040这三处为纯文本步骤,补上 “text” 语言标识,保持与全篇一致的规范。
-``` +```text 开始 → Loading → 随机书签列表 → 操作书签 → 完成庆祝 → 再来一组 -``` +``` -``` +```text 开始 → Loading → 历史书签列表 → 继续操作 → 完成庆祝 -``` +``` -``` +```text 开始 → Loading → 错误页面 → 重试 → 正常流程 -``` +```
145-150: CommandBuilder 状态描述与代码变更保持一致,建议强调 lastValue 判定此处已采用 “if lastValue.isEmpty” 的新语义,建议在文档上补一句“前提:lastValue != null”,以避免误解空引用情况。
-├── whileExecuting → Loading组件 (if lastValue.isEmpty) +├── whileExecuting → Loading组件(前提:lastValue != null,且 lastValue.isEmpty)test/ui/daily_read/view_models/daily_read_viewmodel_test.mocks.dart (1)
45-49: 提示:这是 Mockito 生成文件,请勿手改;确保测试桩返回值类型与接口一致此文件对仓库接口的返回类型具备“权威性”。例如:
- BookmarkRepository.toggleArchived/Marked → Future<ResultDart<void, Exception>>
- BookmarkOperationUseCases.openUrl → Future<ResultDart<void, Exception>>
请在测试桩中严格返回
ResultDart<void, Exception>,否则将出现泛型不匹配导致的编译错误。test/ui/daily_read/view_models/daily_read_viewmodel_test.dart (1)
349-384: “归档回调”用例断言较弱,建议补充可观测性或移除占位断言当前仅设置回调并断言
expect(true, true),无法证明回调行为。建议:
- 若 ViewModel 提供可触发归档完成的公开方法或事件,调用之并断言回调被触发;
- 或通过在 ViewModel 内暴露只读的回调是否已设置状态(供测试观察);
- 若暂不可测,建议删除占位断言,改为验证“设置/清除回调”不抛异常的语义测试。
我可以帮你改造该用例,使其在不破坏封装的前提下具备有效断言。
test/ui/daily_read/widgets/daily_read_screen_test.dart (1)
228-270: 测试验证不完整当前测试只验证了
setOnBookmarkArchivedCallback被调用,但没有测试实际的书签操作(如归档、标记、打开URL等)是否正确触发相应的 Command。建议增强测试以验证用户交互:
// 添加测试用户点击归档按钮 testWidgets('should call toggleBookmarkArchived when archive button is tapped', (WidgetTester tester) async { // ... 设置代码 ... await tester.pumpWidget(createWidgetUnderTest()); await tester.pumpAndSettle(); // 查找并点击归档按钮 final archiveButton = find.byIcon(Icons.archive); expect(archiveButton, findsOneWidget); await tester.tap(archiveButton); // 验证 Command 被执行 verify(mockDailyReadViewModel.toggleBookmarkArchived) .execute(testBookmark.bookmark); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
CLAUDE.md(1 hunks)docs/daily_read_feature_specification.md(1 hunks)docs/daily_read_interaction_flows.md(1 hunks)lib/ui/daily_read/widgets/daily_read_screen.dart(1 hunks)test/ui/daily_read/view_models/daily_read_viewmodel_test.dart(1 hunks)test/ui/daily_read/view_models/daily_read_viewmodel_test.mocks.dart(1 hunks)test/ui/daily_read/widgets/daily_read_screen_test.dart(1 hunks)test/ui/daily_read/widgets/daily_read_screen_test.mocks.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
📄 CodeRabbit Inference Engine (.trae/rules/project_rules.md)
所有样式必须通过主题提供,不允许硬编码颜色、字号等(例如禁止直接使用 Color/0xFF/固定 fontSize)
Files:
lib/ui/daily_read/widgets/daily_read_screen.dart
lib/ui/**/*[Ss]creen.dart
📄 CodeRabbit Inference Engine (.trae/rules/project_rules.md)
lib/ui/**/*[Ss]creen.dart: View(Screen)构造函数仅接受 key 与 viewModel;不包含任何业务逻辑;用户交互通过 Command 执行;处理 Command 的执行中/错误/完成状态
在 StatefulWidget 中通过 Command 监听器处理完成与错误:initState 中订阅 results/errors,dispose 中取消,使用 mounted 检查
所有错误状态应统一使用 ErrorPage 组件,优先通过工厂方法(如 ErrorPage.fromException)创建并遵循主题
Files:
lib/ui/daily_read/widgets/daily_read_screen.dart
🧠 Learnings (6)
📚 Learning: 2025-08-13T18:04:36.250Z
Learnt from: CR
PR: shadowfish07/ReadeckAPP#0
File: .trae/rules/project_rules.md:0-0
Timestamp: 2025-08-13T18:04:36.250Z
Learning: Applies to lib/**/*ViewModel.dart : 对外暴露的异步操作通过 flutter_command 的 Command 暴露;UI 状态采用 private setter + public getter
Applied to files:
lib/ui/daily_read/widgets/daily_read_screen.dart
📚 Learning: 2025-08-13T18:04:36.250Z
Learnt from: CR
PR: shadowfish07/ReadeckAPP#0
File: .trae/rules/project_rules.md:0-0
Timestamp: 2025-08-13T18:04:36.250Z
Learning: Applies to lib/**/*ViewModel.dart : ViewModel 通过私有 final 成员持有 Repository 引用(不直接依赖 Service),并在构造函数中初始化所有 Commands
Applied to files:
lib/ui/daily_read/widgets/daily_read_screen.dart
📚 Learning: 2025-08-13T18:04:36.250Z
Learnt from: CR
PR: shadowfish07/ReadeckAPP#0
File: .trae/rules/project_rules.md:0-0
Timestamp: 2025-08-13T18:04:36.250Z
Learning: Applies to test/unit/ui/**/*.dart : ViewModel 层需具备 100% 单元测试覆盖,并包含 Result 处理逻辑测试
Applied to files:
test/ui/daily_read/view_models/daily_read_viewmodel_test.darttest/ui/daily_read/widgets/daily_read_screen_test.dart
📚 Learning: 2025-08-13T18:04:36.250Z
Learnt from: CR
PR: shadowfish07/ReadeckAPP#0
File: .trae/rules/project_rules.md:0-0
Timestamp: 2025-08-13T18:04:36.250Z
Learning: Applies to test/unit/data/**/*.dart : Repository 层需具备 100% 单元测试覆盖
Applied to files:
test/ui/daily_read/view_models/daily_read_viewmodel_test.darttest/ui/daily_read/widgets/daily_read_screen_test.dart
📚 Learning: 2025-08-13T18:04:36.250Z
Learnt from: CR
PR: shadowfish07/ReadeckAPP#0
File: .trae/rules/project_rules.md:0-0
Timestamp: 2025-08-13T18:04:36.250Z
Learning: Applies to test/widget/**/*.dart : 为关键 Widget 提供 UI 测试
Applied to files:
test/ui/daily_read/widgets/daily_read_screen_test.dart
📚 Learning: 2025-08-13T18:04:36.250Z
Learnt from: CR
PR: shadowfish07/ReadeckAPP#0
File: .trae/rules/project_rules.md:0-0
Timestamp: 2025-08-13T18:04:36.250Z
Learning: Applies to test/integration/**/*.dart : 编写集成测试覆盖核心用户流程与 Command 执行链路
Applied to files:
test/ui/daily_read/widgets/daily_read_screen_test.dart
🪛 LanguageTool
docs/daily_read_feature_specification.md
[uncategorized] ~21-~21: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:固定"地"阅读
Context: ...缺乏动力: 没有阅读进度反馈和成就感 3. 习惯缺失: 没有固定的阅读节奏和提醒 4. 内容分散: 书签管理和阅读体验割裂 ### 解决...
(wb4)
[uncategorized] ~378-~378: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:清晰"地"加
Context: ...📱 用户体验设计 ### 视觉反馈 1. Loading状态: 清晰的加载指示器 2. 操作反馈: 按钮点击状态变化 3. 完成庆祝:...
(wb4)
docs/daily_read_interaction_flows.md
[grammar] ~3-~3: There might be a mistake here.
Context: # 每日阅读交互流程图 ## 🔄 核心交互流程 ### 1. 数据加载流程 ```mermaid flowchart TD A[用...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...I->>UI: 显示错误页面 end ## 📱 用户交互路径 ### 路径A: 首次使用 开始 → Loading → 随机书签列表 → 操作书...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...ding → 错误页面 → 重试 → 正常流程 ``` ## 🎮 操作映射表 | 用户操作 | UI组件 | ViewModel方法 | Repository方...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...组件 | ViewModel方法 | Repository方法 | 结果反馈 | |---------|-------|---------------|-----...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...-----------|----------------|----------| | 打开页面 | DailyReadScreen | load.execute(...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...adRandomUnarchivedBookmarks() | 显示书签列表 | | 归档书签 | BookmarkCard | toggleBookmarkAr...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...hived | toggleArchived() | UI刷新+可能触发庆祝 | | 标记书签 | BookmarkCard | toggleBookmarkMa...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...BookmarkMarked | toggleMarked() | UI刷新 | | 打开URL | BookmarkCard | openUrl | openU...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...markCard | openUrl | openUrl() | 打开浏览器 | | 编辑标签 | BookmarkCard | updateBookmarkLa...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...BookmarkLabels | updateLabels() | UI刷新 | | 点击卡片 | BookmarkCard | - | - | 导航到详情页 |...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...| 点击卡片 | BookmarkCard | - | - | 导航到详情页 | | 再来一组 | CelebrationOverlay | load.execu...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...loadRandomUnarchivedBookmarks() | 重新加载 | | 刷新 | ErrorPage | load.execute(false) |...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...chivedBookmarks() | 重试加载 | ## 🔧 状态管理详解 ### ViewModel状态属性 ```dart // 私有状态 List<Bookma...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...kmarkArchivedCallback; ## 🎨 UI渲染逻辑 ### render()方法决策树 render() ├── unArchived...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...r) └── onData → render() ## 📊 数据流图 ### 完整数据流mermaid graph TD A[用户操作] -->...
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ...-> I[状态更新] I --> D ## 🔄 生命周期管理 ### 页面生命周期dart initState() { // 初始化礼花控制...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/daily_read_feature_specification.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
89-89: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
152-152: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
161-161: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
270-270: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
390-390: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/daily_read_interaction_flows.md
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
135-135: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: review-pr
🔇 Additional comments (5)
docs/daily_read_interaction_flows.md (1)
87-97: 确认:文档中updateBookmarkLabels在代码中存在,文档与实现一致已检索到该方法的声明与调用,关键位置如下:
- lib/ui/daily_read/view_models/daily_read_viewmodel.dart(声明)
- lib/ui/bookmarks/view_models/bookmarks_viewmodel.dart(声明)
- lib/ui/bookmarks/view_models/bookmark_detail_viewmodel.dart(声明)
- lib/ui/daily_read/widgets/daily_read_screen.dart(调用)
- lib/ui/bookmarks/widget/bookmark_list_screen.dart(调用)
- lib/ui/bookmarks/widget/bookmark_detail_screen.dart(调用)
- lib/data/repository/bookmark/bookmark_repository.dart(仓库层对应方法为
updateLabels)结论:文档中使用
updateBookmarkLabels无误,和代码实现一致,无需修改。test/ui/daily_read/view_models/daily_read_viewmodel_test.dart (3)
78-134: 首次加载用例整体 LGTM,包含历史查询与随机加载分支桩测试覆盖了“无当日历史 → 随机加载 5 条 → 保存历史”的主路径,并校验了交互次数。等待
Future.delayed(Duration.zero)的处理也合理。
135-195: 从历史加载用例 LGTM,断言与 never 调用校验完善验证了命中当日历史时不再触发随机加载,状态断言清晰。
315-347: 标签加载用例 LGTM桩与断言匹配预期路径:调用
loadLabels()后通过labelNames映射得到展示所需的字符串集合。test/ui/daily_read/widgets/daily_read_screen_test.mocks.dart (1)
1-251: 自动生成的 Mock 文件正确这是 Mockito 自动生成的 mock 文件,代码结构正确,包含了所有必要的 mock 实现。
| command: viewModel.load, | ||
| whileExecuting: (context, lastValue, param) { | ||
| if (lastValue == null) { | ||
| if (lastValue != null && lastValue.isEmpty) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
加载条件逻辑可能存在问题
当前实现中,只有当 lastValue 非空且为空列表时才显示加载提示。这意味着首次加载(lastValue 为 null)时不会显示加载提示,这可能不是预期行为。
建议修改为同时处理 null 和空列表的情况:
- if (lastValue != null && lastValue.isEmpty) {
+ if (lastValue == null || lastValue.isEmpty) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (lastValue != null && lastValue.isEmpty) { | |
| if (lastValue == null || lastValue.isEmpty) { |
🤖 Prompt for AI Agents
In lib/ui/daily_read/widgets/daily_read_screen.dart around line 232, the
loading-condition currently checks only for non-null but empty list (if
(lastValue != null && lastValue.isEmpty)), which skips showing the loader on the
initial load when lastValue is null; change the condition to treat both null and
empty as "loading" (for example use if (lastValue == null || lastValue.isEmpty)
or the equivalent null-aware check) and ensure lastValue's nullable type is
handled safely so the isEmpty check does not cause a null access.
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:mockito/annotations.dart'; | ||
| import 'package:mockito/mockito.dart'; | ||
| import 'package:readeck_app/data/repository/bookmark/bookmark_repository.dart'; | ||
| import 'package:readeck_app/data/repository/daily_read_history/daily_read_history_repository.dart'; | ||
| import 'package:readeck_app/data/repository/label/label_repository.dart'; | ||
| import 'package:readeck_app/domain/models/bookmark/bookmark.dart'; | ||
| import 'package:readeck_app/domain/models/bookmark/label_info.dart'; | ||
| import 'package:readeck_app/domain/models/bookmark_display_model/bookmark_display_model.dart'; | ||
| import 'package:readeck_app/domain/models/daily_read_history/daily_read_history.dart'; | ||
| import 'package:readeck_app/domain/use_cases/bookmark_operation_use_cases.dart'; | ||
| import 'package:readeck_app/ui/daily_read/view_models/daily_read_viewmodel.dart'; | ||
| import 'package:logger/logger.dart'; | ||
| import 'package:readeck_app/main.dart'; | ||
| import 'package:readeck_app/utils/option_data.dart'; | ||
| import 'package:result_dart/result_dart.dart'; | ||
|
|
||
| import 'daily_read_viewmodel_test.mocks.dart'; | ||
|
|
||
| @GenerateMocks([ | ||
| BookmarkRepository, | ||
| DailyReadHistoryRepository, | ||
| BookmarkOperationUseCases, | ||
| LabelRepository | ||
| ]) | ||
| void main() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
建议补充失败路径测试以满足项目规则“ViewModel 层需包含 Result 处理逻辑测试”
根据项目规则与本 PR 目标,建议新增失败场景测试,例如:
- 历史查询失败(Failure)
- 随机加载失败(Failure)
- toggle 操作失败(Failure)
- openUrl 失败(Failure)
并断言错误传播或 UI 层 Command 的 onError 行为契约。
可用如下脚本初步盘点是否已有失败场景用例(查找 Failure/throw/异常 相关断言):
🏁 Script executed:
#!/bin/bash
# 在测试中搜索 ResultDart 的失败使用与错误断言相关线索
rg -nP -C2 '\bFailure\(|throw|onError|errors\.listen' --type=dart test/ui/daily_readLength of output: 2458
需补充失败路径的单元测试(DailyReadViewModel)
简短结论:按项目规则(ViewModel 层需包含 Result 处理逻辑测试),在 test/ui/daily_read 目录中未发现失败场景测试——你提交的 rg 搜索只命中生成的 mock 文件(包含 throwOnMissingStub),没有找到 Failure/throw/onError 相关断言。
待办(请补充这些测试):
- 在 test/ui/daily_read/view_models/daily_read_viewmodel_test.dart 中新增失败场景用例:
- 历史查询失败(Failure)
- 随机加载失败(Failure)
- toggle 操作失败(Failure)
- openUrl 失败(Failure)
- 使用生成的 mocks(MockBookmarkRepository、MockDailyReadHistoryRepository、MockBookmarkOperationUseCases、MockLabelRepository)通过 when(...).thenAnswer(...) 模拟返回 Failure 或抛出异常。
- 断言 ViewModel 的错误传播契约:例如 Command 的 onError 被触发或 ViewModel 更新到预期的 error state / 发出错误事件。
注意:
- 不要修改生成的 mocks 文件(*.mocks.dart);在测试内模拟行为即可。
- 这些测试是满足项目规则并防止回归所必需的。
🤖 Prompt for AI Agents
In test/ui/daily_read/view_models/daily_read_viewmodel_test.dart lines 1-26, the
test file lacks unit tests for failure paths of DailyReadViewModel; add tests
that simulate failures for history fetch, random load, toggle operation, and
openUrl using the generated mocks (MockBookmarkRepository,
MockDailyReadHistoryRepository, MockBookmarkOperationUseCases,
MockLabelRepository). For each case, stub the mock methods with
when(...).thenAnswer(...) to return a Failure Result or throw an exception, then
assert the ViewModel reacts correctly by triggering the Command onError callback
or updating to the expected error state / emitting an error event; do not modify
any *.mocks.dart files and keep test assertions aligned with the project's
Result handling conventions.
| provideDummy<ResultDart<void, Exception>>( | ||
| const Success('unit'), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResultDart<void, Exception> 的桩值应返回 void 成功,避免类型不匹配
当前使用 const Success('unit') 作为 void 结果的 dummy,存在类型不匹配的隐患。建议改为 const Success(null)(或项目中约定的 unit 模式)以与 ResultDart<void, Exception> 对齐。
- provideDummy<ResultDart<void, Exception>>(
- const Success('unit'),
- );
+ provideDummy<ResultDart<void, Exception>>(
+ const Success(null),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| provideDummy<ResultDart<void, Exception>>( | |
| const Success('unit'), | |
| ); | |
| provideDummy<ResultDart<void, Exception>>( | |
| const Success(null), | |
| ); |
🤖 Prompt for AI Agents
In test/ui/daily_read/view_models/daily_read_viewmodel_test.dart around lines
45–47, the stub value provided for ResultDart<void, Exception> uses const
Success('unit') which is a String and mismatches the void result type; replace
that with a void-compatible success such as const Success(null) (or the
project's canonical unit value) so the dummy aligns with ResultDart<void,
Exception> and update any related tests to expect the unit/null value.
| when(mockBookmarkRepository.toggleArchived(testBookmark)) | ||
| .thenAnswer((_) async => Success(testBookmark)); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
桩返回值类型与仓库接口不一致(toggleArchived)
Mock 定义返回 Future<ResultDart<void, Exception>>,但这里返回了 Success(testBookmark)(ResultDart<Bookmark, Exception>)。应改为 void 成功。
- when(mockBookmarkRepository.toggleArchived(testBookmark))
- .thenAnswer((_) async => Success(testBookmark));
+ when(mockBookmarkRepository.toggleArchived(testBookmark))
+ .thenAnswer((_) async => const Success(null));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| when(mockBookmarkRepository.toggleArchived(testBookmark)) | |
| .thenAnswer((_) async => Success(testBookmark)); | |
| when(mockBookmarkRepository.toggleArchived(testBookmark)) | |
| .thenAnswer((_) async => const Success(null)); |
🤖 Prompt for AI Agents
In test/ui/daily_read/view_models/daily_read_viewmodel_test.dart around lines
220 to 222, the mock's stub for toggleArchived currently returns
Success(testBookmark) which has type ResultDart<Bookmark, Exception> but the
repository interface expects Future<ResultDart<void, Exception>>; update the
stub to return a void-success value (e.g., Success(null)) wrapped as the
expected Future so the returned ResultDart matches ResultDart<void, Exception>.
| when(mockBookmarkRepository.toggleMarked(testBookmark)) | ||
| .thenAnswer((_) async => Success(testBookmark)); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
桩返回值类型与仓库接口不一致(toggleMarked)
与上同理,应返回 ResultDart<void, Exception> 的成功。
- when(mockBookmarkRepository.toggleMarked(testBookmark))
- .thenAnswer((_) async => Success(testBookmark));
+ when(mockBookmarkRepository.toggleMarked(testBookmark))
+ .thenAnswer((_) async => const Success(null));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| when(mockBookmarkRepository.toggleMarked(testBookmark)) | |
| .thenAnswer((_) async => Success(testBookmark)); | |
| when(mockBookmarkRepository.toggleMarked(testBookmark)) | |
| .thenAnswer((_) async => const Success(null)); |
🤖 Prompt for AI Agents
In test/ui/daily_read/view_models/daily_read_viewmodel_test.dart around lines
263 to 265, the mock for toggleMarked returns a Success containing a Bookmark
but the repository interface expects ResultDart<void, Exception>; change the
stubbed return to a void-success ResultDart (e.g. Success<void, Exception>(null)
or equivalent) so the mocked return type matches the repository signature.
| when(mockBookmarkOperationUseCases.openUrl(testUrl)) | ||
| .thenAnswer((_) async => const Success(true)); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openUrl 的桩返回值类型不匹配,应返回 void 成功
openUrl 的接口是 Future<ResultDart<void, Exception>>,当前返回 const Success(true)(ResultDart<bool, Exception>)。请改为 void 成功。
- when(mockBookmarkOperationUseCases.openUrl(testUrl))
- .thenAnswer((_) async => const Success(true));
+ when(mockBookmarkOperationUseCases.openUrl(testUrl))
+ .thenAnswer((_) async => const Success(null));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| when(mockBookmarkOperationUseCases.openUrl(testUrl)) | |
| .thenAnswer((_) async => const Success(true)); | |
| when(mockBookmarkOperationUseCases.openUrl(testUrl)) | |
| .thenAnswer((_) async => const Success(null)); |
🤖 Prompt for AI Agents
In test/ui/daily_read/view_models/daily_read_viewmodel_test.dart around lines
296–298, the mock for openUrl returns a Success wrapping a bool (true) but the
method's signature is Future<ResultDart<void, Exception>>; change the stub to
return a Success representing a void success (i.e., the Success value should be
the void/null variant) so the mocked return type matches ResultDart<void,
Exception> and the test compiles.
| group('DailyReadScreen', () { | ||
| testWidgets('should display bookmarks when viewmodel has data', | ||
| (WidgetTester tester) async { | ||
| // Arrange | ||
| final bookmarks = [ | ||
| BookmarkDisplayModel( | ||
| bookmark: Bookmark( | ||
| id: '1', | ||
| url: '', | ||
| title: 'First Bookmark Title', | ||
| isArchived: false, | ||
| isMarked: false, | ||
| labels: [], | ||
| created: DateTime.now(), | ||
| readProgress: 0)), | ||
| BookmarkDisplayModel( | ||
| bookmark: Bookmark( | ||
| id: '2', | ||
| url: '', | ||
| title: 'Second Bookmark Title', | ||
| isArchived: false, | ||
| isMarked: false, | ||
| labels: [], | ||
| created: DateTime.now(), | ||
| readProgress: 0)), | ||
| ]; | ||
| final loadCommand = Command.createAsync<bool, List<BookmarkDisplayModel>>( | ||
| (_) async => bookmarks, | ||
| initialValue: []); | ||
|
|
||
| when(mockDailyReadViewModel.unArchivedBookmarks).thenReturn(bookmarks); | ||
| when(mockDailyReadViewModel.load).thenReturn(loadCommand); | ||
| when(mockDailyReadViewModel.isNoMore).thenReturn(false); | ||
| when(mockDailyReadViewModel.availableLabels).thenReturn([]); | ||
| when(mockDailyReadViewModel.getReadingStats(any)).thenReturn(null); | ||
|
|
||
| // Act | ||
| await tester.pumpWidget(createWidgetUnderTest()); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| // Assert | ||
| expect(find.text('First Bookmark Title'), findsOneWidget); | ||
| expect(find.text('Second Bookmark Title'), findsOneWidget); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
测试覆盖率不足:缺少错误处理测试
当前测试套件没有覆盖错误处理场景,如网络错误或未知错误的显示。虽然注释中提到错误测试复杂,但这些是关键路径,应该被测试覆盖。
建议添加错误场景测试:
testWidgets('should display network error page when load fails with NetworkErrorException',
(WidgetTester tester) async {
// Arrange
final loadCommand = Command.createAsync<bool, List<BookmarkDisplayModel>>(
(param) async {
throw NetworkErrorException('Network error');
},
initialValue: null,
);
when(mockDailyReadViewModel.load).thenReturn(loadCommand);
when(mockDailyReadViewModel.unArchivedBookmarks).thenReturn([]);
when(mockDailyReadViewModel.isNoMore).thenReturn(false);
// Act
await tester.pumpWidget(createWidgetUnderTest());
loadCommand.execute(false);
await tester.pumpAndSettle();
// Assert
expect(find.byType(ErrorPage), findsOneWidget);
expect(find.text('网络连接失败'), findsOneWidget);
});🤖 Prompt for AI Agents
In test/ui/daily_read/widgets/daily_read_screen_test.dart around lines 52 to 95,
add a new widget test that covers the error path: create a load Command that
throws a NetworkErrorException (or appropriate error type), stub
mockDailyReadViewModel.load to return that command and unArchivedBookmarks to
return an empty list, pump the widget, execute the load command, await
pumpAndSettle, and assert the ErrorPage (or error widget) is displayed and the
localized network error text appears; ensure the test cleans up mocks and uses
the same createWidgetUnderTest setup so it integrates with existing tests.
| testWidgets('should display loading indicator when loading', | ||
| (WidgetTester tester) async { | ||
| // Arrange | ||
| // Create a real Command that matches the ViewModel configuration | ||
| final loadCommand = Command.createAsync<bool, List<BookmarkDisplayModel>>( | ||
| (param) async { | ||
| // Simulate a short operation so it completes during test | ||
| await Future.delayed(const Duration(milliseconds: 100)); | ||
| return <BookmarkDisplayModel>[]; | ||
| }, | ||
| includeLastResultInCommandResults: true, | ||
| initialValue: [], | ||
| ); | ||
|
|
||
| when(mockDailyReadViewModel.load).thenReturn(loadCommand); | ||
| when(mockDailyReadViewModel.unArchivedBookmarks).thenReturn([]); | ||
| when(mockDailyReadViewModel.isNoMore).thenReturn(false); | ||
| when(mockDailyReadViewModel.availableLabels).thenReturn([]); | ||
|
|
||
| // Start the command to put it in executing state | ||
| loadCommand.execute(false); | ||
|
|
||
| // Act | ||
| await tester.pumpWidget(createWidgetUnderTest()); | ||
|
|
||
| // Wait for the widget to be built and command to start executing | ||
| await tester.pump(const Duration(milliseconds: 10)); | ||
|
|
||
| // Assert - should show loading when command is executing with empty initial value | ||
| expect(find.text('正在加载今日推荐'), findsOneWidget); | ||
|
|
||
| // Wait for the command to complete to avoid pending timer issues | ||
| await tester.pumpAndSettle(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
测试用例与实际加载逻辑不符
测试期望在 lastValue 为空列表时显示加载提示,但根据 daily_read_screen.dart 的最新改动,加载提示只在 lastValue != null && lastValue.isEmpty 时显示。当前测试设置 initialValue: [],这会导致测试可能因时序问题而不稳定。
建议修改测试以匹配实际逻辑:
final loadCommand = Command.createAsync<bool, List<BookmarkDisplayModel>>(
(param) async {
// Simulate a short operation so it completes during test
await Future.delayed(const Duration(milliseconds: 100));
return <BookmarkDisplayModel>[];
},
includeLastResultInCommandResults: true,
- initialValue: [],
+ // 不设置 initialValue,让其默认为 null
);或者如果要测试空列表显示加载的情况,需要先让 Command 返回空列表,然后再次执行。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| testWidgets('should display loading indicator when loading', | |
| (WidgetTester tester) async { | |
| // Arrange | |
| // Create a real Command that matches the ViewModel configuration | |
| final loadCommand = Command.createAsync<bool, List<BookmarkDisplayModel>>( | |
| (param) async { | |
| // Simulate a short operation so it completes during test | |
| await Future.delayed(const Duration(milliseconds: 100)); | |
| return <BookmarkDisplayModel>[]; | |
| }, | |
| includeLastResultInCommandResults: true, | |
| initialValue: [], | |
| ); | |
| when(mockDailyReadViewModel.load).thenReturn(loadCommand); | |
| when(mockDailyReadViewModel.unArchivedBookmarks).thenReturn([]); | |
| when(mockDailyReadViewModel.isNoMore).thenReturn(false); | |
| when(mockDailyReadViewModel.availableLabels).thenReturn([]); | |
| // Start the command to put it in executing state | |
| loadCommand.execute(false); | |
| // Act | |
| await tester.pumpWidget(createWidgetUnderTest()); | |
| // Wait for the widget to be built and command to start executing | |
| await tester.pump(const Duration(milliseconds: 10)); | |
| // Assert - should show loading when command is executing with empty initial value | |
| expect(find.text('正在加载今日推荐'), findsOneWidget); | |
| // Wait for the command to complete to avoid pending timer issues | |
| await tester.pumpAndSettle(); | |
| }); | |
| testWidgets('should display loading indicator when loading', | |
| (WidgetTester tester) async { | |
| // Arrange | |
| // Create a real Command that matches the ViewModel configuration | |
| final loadCommand = Command.createAsync<bool, List<BookmarkDisplayModel>>( | |
| (param) async { | |
| // Simulate a short operation so it completes during test | |
| await Future.delayed(const Duration(milliseconds: 100)); | |
| return <BookmarkDisplayModel>[]; | |
| }, | |
| includeLastResultInCommandResults: true, | |
| // 不设置 initialValue,让其默认为 null | |
| ); | |
| when(mockDailyReadViewModel.load).thenReturn(loadCommand); | |
| when(mockDailyReadViewModel.unArchivedBookmarks).thenReturn([]); | |
| when(mockDailyReadViewModel.isNoMore).thenReturn(false); | |
| when(mockDailyReadViewModel.availableLabels).thenReturn([]); | |
| // Start the command to put it in executing state | |
| loadCommand.execute(false); | |
| // Act | |
| await tester.pumpWidget(createWidgetUnderTest()); | |
| // Wait for the widget to be built and command to start executing | |
| await tester.pump(const Duration(milliseconds: 10)); | |
| // Assert - should show loading when command is executing with empty initial value | |
| expect(find.text('正在加载今日推荐'), findsOneWidget); | |
| // Wait for the command to complete to avoid pending timer issues | |
| await tester.pumpAndSettle(); | |
| }); |
🤖 Prompt for AI Agents
In test/ui/daily_read/widgets/daily_read_screen_test.dart around lines 97–130,
the test assumes a loading indicator is shown when lastValue is an empty list,
but the screen now shows loading only when lastValue != null &&
lastValue.isEmpty; update the test to match that logic by either (A) set the
Command's initialValue to null and change the assertion to expect no loading, or
(B) if you want to test the empty-list-loading case, have the Command first
return an empty list (so lastValue becomes []) and then execute it again to
enter the executing state before pumping the widget and asserting the loading
indicator; pick one approach and adjust the test assertions and Command setup
accordingly.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit