-
Notifications
You must be signed in to change notification settings - Fork 373
chore(core, ui): fix lints and failing tests #2376
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
WalkthroughUpdated an import to hide Flutter's Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial 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). (9)
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 |
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: 0
🧹 Nitpick comments (1)
packages/stream_chat_flutter_core/lib/src/paged_value_notifier.dart (1)
1-1: Prefer selectiveshowoverhidefor clarity and narrower surfaceHiding
Keyfixes the lint, but importing only what’s used keeps the namespace clean and avoids future surprises.-import 'package:flutter/widgets.dart' hide Key; +import 'package:flutter/widgets.dart' show ValueListenableBuilder, ValueNotifier;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/stream_chat_flutter_core/lib/src/paged_value_notifier.dart(1 hunks)
⏰ 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). (11)
- GitHub Check: update_goldens
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_flutter_core
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: analyze
- GitHub Check: analyze_legacy_versions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2376 +/- ##
==========================================
- Coverage 63.80% 63.77% -0.04%
==========================================
Files 412 412
Lines 25824 25824
==========================================
- Hits 16478 16469 -9
- Misses 9346 9355 +9 ☔ 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: 0
🧹 Nitpick comments (2)
packages/stream_chat_flutter/test/src/fakes.dart (1)
134-136: Return a real temp file from stop() to avoid downstream file-existence assumptions.Some code paths may
File(path).existsSync()or try to read metadata. Safer to create an empty temp file and return its path.Apply this diff within
stop:- Future<String?> stop(String recorderId) async { - return 'path'; - } + Future<String?> stop(String recorderId) async { + final ts = DateTime.now().microsecondsSinceEpoch; + final file = File('${Directory.systemTemp.path}/fake_record_$ts.m4a'); + if (!await file.exists()) { + await file.create(recursive: true); + } + return file.path; + }Add this import (outside the selected range):
import 'dart:io';packages/stream_chat_flutter/test/src/message_input/message_input_test.dart (1)
16-19: Initialize test binding at start of main for consistency.Other suites call
TestWidgetsFlutterBinding.ensureInitialized();. Add it here to avoid sporadic platform-channel init issues on some runners.void main() { + TestWidgetsFlutterBinding.ensureInitialized(); final originalRecordPlatform = RecordPlatform.instance; setUp(() => RecordPlatform.instance = FakeRecordPlatform()); tearDown(() => RecordPlatform.instance = originalRecordPlatform);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (30)
packages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_option_reorderable_list_view_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_option_reorderable_list_view_error_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_option_reorderable_list_view_error_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_option_reorderable_list_view_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_question_text_field_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_question_text_field_error_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_question_text_field_error_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/poll_question_text_field_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/stream_poll_creator_dialog_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/stream_poll_creator_dialog_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/stream_poll_creator_full_screen_dialog_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/creator/goldens/ci/stream_poll_creator_full_screen_dialog_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/poll_option_reorderable_list_view_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/poll_option_reorderable_list_view_error.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/poll_option_reorderable_list_view_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/poll_question_text_field_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/poll_question_text_field_error.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/poll_question_text_field_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/stream_poll_creator_dialog_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/stream_poll_creator_dialog_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/stream_poll_creator_full_screen_dialog_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/goldens/ci/stream_poll_creator_full_screen_dialog_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_add_comment_dialog_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_add_comment_dialog_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_add_comment_dialog_with_initial_value_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_add_comment_dialog_with_initial_value_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_suggest_option_dialog_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_suggest_option_dialog_light.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_suggest_option_dialog_with_initial_option_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/poll/interactor/goldens/ci/poll_suggest_option_dialog_with_initial_option_light.pngis excluded by!**/*.png
📒 Files selected for processing (4)
packages/stream_chat_flutter/test/src/bottom_sheets/edit_message_sheet_test.dart(1 hunks)packages/stream_chat_flutter/test/src/fakes.dart(2 hunks)packages/stream_chat_flutter/test/src/message_actions_modal/message_actions_modal_test.dart(1 hunks)packages/stream_chat_flutter/test/src/message_input/message_input_test.dart(1 hunks)
⏰ 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). (12)
- GitHub Check: update_goldens
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat
- GitHub Check: stream_chat_persistence
- GitHub Check: test
- GitHub Check: build (ios)
- GitHub Check: format
- GitHub Check: analyze
- GitHub Check: build (android)
- GitHub Check: analyze_legacy_versions
🔇 Additional comments (13)
packages/stream_chat_flutter/test/src/fakes.dart (3)
4-4: LGTM — correct dependency on the platform interface.Using
record_platform_interfacefor the fake keeps tests decoupled from the concrete plugin.
106-143: Solid platform fake; token-safe and covers common calls.
MockPlatformInterfaceMixinensures the token check passes when swappingRecordPlatform.instance.
106-143: Verify FakeRecordPlatform implements any RecordPlatform methods the real AudioRecorder will call (e.g. start / amplitude APIs)
- FakeRecordPlatform is defined at packages/stream_chat_flutter/test/src/fakes.dart; tests set RecordPlatform.instance = FakeRecordPlatform() in:
packages/stream_chat_flutter/test/src/message_input/message_input_test.dart,
packages/stream_chat_flutter/test/src/message_actions_modal/message_actions_modal_test.dart,
packages/stream_chat_flutter/test/src/bottom_sheets/edit_message_sheet_test.dart.- AudioRecorder usage is in packages/stream_chat_flutter/lib/src/message_input/audio_recorder/audio_recorder_controller.dart (calls _recorder.hasPermission(), start(...), stop(), cancel(), dispose()); the controller tests use a MockAudioRecorder so they don’t hit the platform stub.
If any tests or runtime code instantiate the real AudioRecorder (AudioRecorder()) or exercise recording flows, add no-op stubs for RecordPlatform.start and any amplitude-related platform APIs to FakeRecordPlatform; otherwise no change needed.
packages/stream_chat_flutter/test/src/bottom_sheets/edit_message_sheet_test.dart (3)
5-5: LGTM — importingpackage:record/record.dartis appropriate for accessingRecordPlatform.
8-8: LGTM — centralizing fakes via../fakes.dartkeeps tests DRY.
13-18: Good harness for platform swap; isolation is preserved.
ensureInitialized, per-test override, and teardown back to the original instance are all correct.packages/stream_chat_flutter/test/src/message_input/message_input_test.dart (2)
8-8: LGTM — record import added.
12-12: LGTM — shared fake import.packages/stream_chat_flutter/test/src/message_actions_modal/message_actions_modal_test.dart (5)
4-4: LGTM — record import added.
8-8: LGTM — fake import wired.
12-13: LGTM — explicit test binding init.
16-18: LGTM — harmless formatting change in registerFallbackValue.
22-25: Good per-test platform override and restoration.Keeps tests isolated and avoids bleed-through across groups.
This commit introduces a mock for `RecordPlatform` to be used in various tests. The `FakeRecordPlatform` provides default implementations for the `RecordPlatform` interface methods, ensuring that tests relying on audio recording functionality can run without needing actual platform-specific recording capabilities. This mock has been integrated into the following test files: - `message_input_test.dart` - `edit_message_sheet_test.dart` - `message_actions_modal_test.dart` In each of these files, the `FakeRecordPlatform` is set as the `RecordPlatform.instance` during `setUp` and restored to the original platform during `tearDown`.
ee0d207 to
dcbc5e1
Compare
Summary by CodeRabbit
Chores
Refactor
Bug Fixes
Tests
No user-facing behavior changes.