Conversation
thuyenh1081
requested review from
dab246,
hoangdat,
nqhhdev and
tddang-linagora
as code owners
thuyenh1081
had a problem deploying
to
PR-2670
GitHub Actions
Failure
thuyenh1081
force-pushed
the
updateTcOfChatGroupTestDart
branch
from
70d045e to
15ec44b
Compare
WalkthroughThis PR updates integration tests and robots: removes the GroupTest entry from integration_test/.env; adds a public enum Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
integration_test/robots/profile_information_robot.dart (1)
30-42: Index-based text element access may be fragile.The
getDisplayName()logic relies on counting Text elements and using positional access, which could break if the UI structure changes. Similarly,getOnlineStatus()always returnsText.at(1)even when the user might be offline (no status shown).Consider adding defensive checks or using more specific widget keys/semantics for reliable element identification if available.
integration_test/scenarios/chat_detail_scenario.dart (3)
17-28: Avoid hardcoded delays in integration tests.The
Future.delayed(const Duration(seconds: 2))on line 27 is unnecessary sincewaitForEitherVisiblealready ensures results are displayed. Hardcoded delays make tests slower and can still be flaky if the actual wait time varies.🔎 Proposed fix
await ChatGroupDetailRobot($).waitForEitherVisible( $: $, first: $(TwakeListItem), second: $("No Results"), timeout: const Duration(seconds: 10), ); - await Future.delayed(const Duration(seconds: 2)); }
46-63: Potential index-out-of-bounds and misleading variable name.Two concerns:
Line 48:
row.$(Text).at(1)assumes at least 2 Text widgets exist in the row. If a row has fewer Text widgets, this may fail or return unexpected results. Consider adding a guard or using a more robust selector.The variable
owner(line 48) is misleading since it holds the user level string (which could be "admin", "member", etc.), not specifically an owner. Consider renaming touserLevelTextfor clarity.Line 62: The hardcoded delay with comment suggests a synchronization issue. Consider investigating if there's a widget state change that can be waited on instead.
🔎 Suggested variable rename
for (final item in items) { final row = item.root; - final owner = row.$(Text).at(1).text; + final userLevelText = row.$(Text).at(1).text; final displayName = row.$(Text).first.text ?? ""; final matrixAddress = row.$(Text).last.text ?? ""; await verifyProfileInfoOfAMember( s, displayName, matrixAddress, isCurrentUser: matrixAddress == _currentAccount, - level: parseUserLevel(owner), + level: parseUserLevel(userLevelText), );
105-113: Try-catch around soft assertion may hide test issues.The try-catch here defeats the purpose of soft assertions. If
.existscan throw an exception (rather than returningfalse), catching and silently ignoring it means:
- Actual errors (like selector issues) are hidden
- The test won't report when online status is missing
If online status is truly optional, the soft assertion alone should handle it gracefully. If
.existsthrows when the element isn't found, that's a bug in the finder that should be fixed at the source.🔎 Proposed fix
- try { - s.softAssertEquals( - ProfileInformationRobot($).getOnlineStatus().exists, - true, - "online status is not shown", - ); - } catch (_) { - // Ignore: Online status element may not exist for some users - } + // Online status is optional for some users - only assert if expected + // Consider adding an `isOnline` parameter if this should be verified conditionallyOr if online status should always be checked but failures are acceptable:
- try { - s.softAssertEquals( - ProfileInformationRobot($).getOnlineStatus().exists, - true, - "online status is not shown", - ); - } catch (_) { - // Ignore: Online status element may not exist for some users - } + s.softAssertEquals( + ProfileInformationRobot($).getOnlineStatus().exists, + true, + "online status is not shown (optional for some users)", + );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
integration_test/.envintegration_test/base/base_scenario.dartintegration_test/robots/chat_group_detail_robot.dartintegration_test/robots/group_information_robot.dartintegration_test/robots/login_robot.dartintegration_test/robots/profile_information_robot.dartintegration_test/scenarios/chat_detail_scenario.dartintegration_test/scenarios/chat_scenario.dartintegration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/tests/chat/chat_group_test.dart
💤 Files with no reviewable changes (1)
- integration_test/.env
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-09T06:19:59.615Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2761
File: lib/pages/contacts_tab/widgets/sliver_invite_friend_button.dart:49-57
Timestamp: 2025-12-09T06:19:59.615Z
Learning: For the Twake on Matrix Flutter app (fluffychat), desktop platforms (macOS, Linux, Windows) are not supported. During reviews, assume only web and mobile targets and avoid introducing desktop-specific builds, dependencies, or platform checks. This guideline applies across Dart source files in the repository.
Applied to files:
integration_test/robots/login_robot.dartintegration_test/robots/group_information_robot.dartintegration_test/base/base_scenario.dartintegration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/scenarios/chat_scenario.dartintegration_test/robots/profile_information_robot.dartintegration_test/tests/chat/chat_group_test.dartintegration_test/scenarios/chat_detail_scenario.dartintegration_test/robots/chat_group_detail_robot.dart
📚 Learning: 2025-12-22T06:44:09.742Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2644
File: lib/pages/chat/events/audio_message/audio_player_widget.dart:134-134
Timestamp: 2025-12-22T06:44:09.742Z
Learning: When using MatrixFile from matrix-dart-sdk (v4.1.0+), MatrixFile.bytes is non-nullable (Uint8List). Ensure code handles non-null bytes and rely on methods like downloadAndDecryptAttachment() that guarantee non-null returned bytes. Do not assume MatrixFile.bytes may be null; add null-safety checks only where the API guarantees non-null yield, and avoid nullable casts unless upgrading the SDK version. This guideline applies to Dart files across the codebase that interact with MatrixFile.
Applied to files:
integration_test/robots/login_robot.dartintegration_test/robots/group_information_robot.dartintegration_test/base/base_scenario.dartintegration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/scenarios/chat_scenario.dartintegration_test/robots/profile_information_robot.dartintegration_test/tests/chat/chat_group_test.dartintegration_test/scenarios/chat_detail_scenario.dartintegration_test/robots/chat_group_detail_robot.dart
⏰ 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). (3)
- GitHub Check: Deploy preview versions on pull requests
- GitHub Check: Analyze code
- GitHub Check: Widget test
🔇 Additional comments (16)
integration_test/robots/group_information_robot.dart (1)
116-120: Cleaner index-based member enumeration.The refactor from runtime type inspection to index-based
TwakeListItem.at(i)is a good improvement. This approach is more predictable and aligns with standard Patrol patterns for iterating over widget collections.integration_test/robots/login_robot.dart (1)
217-238: Good resilience pattern for CAPTCHA handling.The retry mechanism with a bounded
maxRetrieslimit and clearStateErrormessaging adds robustness to handle flaky CAPTCHA verification flows. The placement of the retry check at the start of the loop body correctly enforces the 10-retry limit.integration_test/base/base_scenario.dart (1)
4-5: Good centralization of UserLevel enum.Moving
UserLevelto the base scenario file makes it accessible across all test scenarios and robots, avoiding duplication. The enum values appropriately model Matrix room permission levels.integration_test/tests/chat/chat_group_test.dart (2)
6-6: Import updated for centralized UserLevel enum.The import of
base_scenario.dartcorrectly provides access to the centralizedUserLevelenum used inverifyTheDisplayOfPullDownMenucalls.
49-56: API updated to message-specific pull-down interactions.The change from
openPullDownMenutoopenPullDownMenuOfAMessageis consistent with the refactored robot API, making the target message explicit rather than relying on implicit context.integration_test/tests/chat/chat_group_open_profile_test.dart (1)
65-79: New test for verifying all member profiles.The test follows established patterns: uses
SoftAssertHelperfor assertion accumulation, navigates through the chat UI, and delegates profile verification toChatDetailScenario. The structure is consistent with existing tests in the file.integration_test/scenarios/chat_scenario.dart (3)
46-47: Improved group chat opening with explicit widget selection.Using
TwakeListItem.containing(groupTitle)provides more explicit targeting, and waiting for the input text field ensures the chat detail view is fully loaded before proceeding.
219-228: Platform-specific delete confirmation handling.The delete flow correctly handles platform differences: iOS uses a text-based
'Delete'selector, while Android uses the more robustclassName+contentDescriptioncombination. This pattern aligns with the existing platform-aware approach in other parts of the codebase.
117-117: Consistent API migration to message-specific pull-down interactions.All
openPullDownMenucalls have been uniformly updated toopenPullDownMenuOfAMessage, maintaining consistency with the refactored robot API across reply, forward, copy, edit, select, pin, unpin, and delete operations.Also applies to: 123-123, 135-135, 179-179, 185-185, 207-207, 212-212, 217-217
integration_test/robots/chat_group_detail_robot.dart (2)
77-79: Synchronous input field accessor.
getInputTextField()correctly returnsPatrolFindersynchronously since the finder operation ($(TextField)) doesn't require async. This simplifies call sites.
94-106: Well-designed pull-down menu API refactoring.The two-method design provides good flexibility:
openPullDownMenuOfAPatrolFinder: Lower-level method accepting aPatrolFinderdirectly for custom targeting.openPullDownMenuOfAMessage: Convenience wrapper for the common case of targeting by message text.Both methods correctly wait for the menu to appear before returning the robot.
integration_test/robots/profile_information_robot.dart (1)
1-69: New profile information robot follows established patterns.The robot properly encapsulates profile screen interactions, extends
CoreRobot, and provides the expected finder methods. ThebackToGroupInformationScreen()navigation helper correctly waits for the target view.integration_test/scenarios/chat_detail_scenario.dart (4)
1-16: LGTM!The imports and class structure follow the established patterns for integration test scenarios.
30-40: LGTM!The parsing logic with null handling and default fallback is well implemented.
66-83: LGTM!Clean and straightforward method that properly coordinates navigation and verification.
121-126: Verify button text: "Sent" vs "Send".The error message references "Sent message button" — is this the actual UI text, or should it be "Send message button" (imperative form)?
|
This PR has been deployed to https://linagora.github.io/twake-on-matrix/2670 |
Insalien
force-pushed
the
updateTcOfChatGroupTestDart
branch
from
15ec44b to
008003e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
integration_test/robots/chat_group_detail_robot.dart (1)
94-101: Consider renaming the parameter for clarity.The parameter name
messagemight be confusing since its type isPatrolFinder, not a message string. Consider renaming tofinder,messageFinder, orelementfor better readability.🔎 Suggested refactor
- Future<PullDownMenuRobot> openPullDownMenuOfAPatrolFinder( - PatrolFinder message, - ) async { - await message.longPress(); + Future<PullDownMenuRobot> openPullDownMenuOfAPatrolFinder( + PatrolFinder finder, + ) async { + await finder.longPress(); await $.waitUntilVisible($(PullDownMenu)); await $.pump(); return PullDownMenuRobot($); }integration_test/robots/profile_information_robot.dart (3)
13-16: Consider usingIconsconstants instead of hardcoded codepoints.Hardcoded
IconDatacodepoints are fragile and may break if the Material Icons font is updated. If these icons correspond to standard Material icons, prefer using theIconsclass constants (e.g.,Icons.copy,Icons.message,Icons.person_remove).
30-38: Selector logic based on Text count may be fragile.The conditional logic determining display name position based on total
Textwidget count could break if the UI structure changes. Consider using a more robust selector, such as finding aTextwidget by a semantic label or key if available.
40-46: Index-based selectors (.at(1)) assume fixed widget ordering.
getOnlineStatus()andgetMatrixAddress()use positional indexing which is fragile. If the UI layout changes, these selectors will silently return wrong elements or throw. Consider adding semantic keys to the widgets under test or using more specific selectors.integration_test/scenarios/chat_detail_scenario.dart (2)
46-50: Extractingownerat index 1 may fail if the widget tree structure varies.Line 48 assumes there's always a
Textwidget at index 1 for the owner. If some member rows don't have an owner label (e.g., regular members without a role badge), this could return incorrect data or throw. Consider adding a guard or using a more specific selector.
27-28: Hardcoded delays reduce test reliability and increase execution time.Using
Future.delayed(Duration(seconds: 2))is a test smell. These delays can still be insufficient on slow devices while unnecessarily slowing down fast environments. Consider using explicit wait conditions likewaitUntilVisibleorpumpAndSettlewith appropriate timeouts, or investigate whypumpAndSettleis insufficient (as noted in the comment on line 61).Also applies to: 61-63
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
integration_test/.envintegration_test/base/base_scenario.dartintegration_test/robots/chat_group_detail_robot.dartintegration_test/robots/group_information_robot.dartintegration_test/robots/login_robot.dartintegration_test/robots/profile_information_robot.dartintegration_test/scenarios/chat_detail_scenario.dartintegration_test/scenarios/chat_scenario.dartintegration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/tests/chat/chat_group_test.dart
💤 Files with no reviewable changes (1)
- integration_test/.env
🚧 Files skipped from review as they are similar to previous changes (3)
- integration_test/robots/group_information_robot.dart
- integration_test/tests/chat/chat_group_test.dart
- integration_test/robots/login_robot.dart
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-09T06:19:59.615Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2761
File: lib/pages/contacts_tab/widgets/sliver_invite_friend_button.dart:49-57
Timestamp: 2025-12-09T06:19:59.615Z
Learning: For the Twake on Matrix Flutter app (fluffychat), desktop platforms (macOS, Linux, Windows) are not supported. During reviews, assume only web and mobile targets and avoid introducing desktop-specific builds, dependencies, or platform checks. This guideline applies across Dart source files in the repository.
Applied to files:
integration_test/robots/chat_group_detail_robot.dartintegration_test/scenarios/chat_detail_scenario.dartintegration_test/robots/profile_information_robot.dartintegration_test/base/base_scenario.dartintegration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/scenarios/chat_scenario.dart
📚 Learning: 2025-12-22T06:44:09.742Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2644
File: lib/pages/chat/events/audio_message/audio_player_widget.dart:134-134
Timestamp: 2025-12-22T06:44:09.742Z
Learning: When using MatrixFile from matrix-dart-sdk (v4.1.0+), MatrixFile.bytes is non-nullable (Uint8List). Ensure code handles non-null bytes and rely on methods like downloadAndDecryptAttachment() that guarantee non-null returned bytes. Do not assume MatrixFile.bytes may be null; add null-safety checks only where the API guarantees non-null yield, and avoid nullable casts unless upgrading the SDK version. This guideline applies to Dart files across the codebase that interact with MatrixFile.
Applied to files:
integration_test/robots/chat_group_detail_robot.dartintegration_test/scenarios/chat_detail_scenario.dartintegration_test/robots/profile_information_robot.dartintegration_test/base/base_scenario.dartintegration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/scenarios/chat_scenario.dart
📚 Learning: 2025-12-09T05:40:51.607Z
Learnt from: nqhhdev
Repo: linagora/twake-on-matrix PR: 2751
File: lib/pages/chat_draft/draft_chat.dart:608-611
Timestamp: 2025-12-09T05:40:51.607Z
Learning: In draft_chat.dart, when sending media with caption through sendImagesWithCaption, the dialog returns SendMediaWithCaptionStatus.emptyRoom (not done) when the user confirms the send, because no room exists yet. The emptyRoom status triggers room creation and media upload, making it effectively the "success" case. Therefore, text should only be restored when status != emptyRoom.
Applied to files:
integration_test/scenarios/chat_scenario.dart
⏰ 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). (3)
- GitHub Check: Deploy preview versions on pull requests
- GitHub Check: Analyze code
- GitHub Check: Widget test
🔇 Additional comments (10)
integration_test/robots/chat_group_detail_robot.dart (3)
77-79: LGTM! Appropriate simplification.Changing
getInputTextField()from async to synchronous is correct since it only constructs and returns a PatrolFinder without performing any async operations.
82-82: LGTM! Consistent with signature change.The removal of
awaitis correct and aligns with the synchronousgetInputTextField()method.
103-107: No further action needed. All usages of the oldopenPullDownMenu(String message)method have been successfully migrated to the newopenPullDownMenuOfAMessage(String message)method throughout the test files.integration_test/base/base_scenario.dart (1)
4-5: LGTM!Centralizing the
UserLevelenum in the base scenario file is a good approach for reusability across different test scenarios. The enum values appropriately cover the expected permission levels.integration_test/tests/chat/chat_group_open_profile_test.dart (1)
66-79: LGTM!The new test for verifying profile information of all members is well-structured. Using
SoftAssertHelperto accumulate assertions before a finalverifyAll()is a good pattern for comprehensive test coverage without early termination on first failure.integration_test/scenarios/chat_scenario.dart (3)
46-48: LGTM!The updated approach using
TwakeListItem.containing(groupTitle)is more explicit and reliable for targeting the correct chat group, and waiting for the input field ensures the chat detail view is fully loaded.
219-228: Platform-specific deletion handling looks correct.iOS uses the native text selector for "Delete", while Android uses a Button with
contentDescription. This is an appropriate pattern for handling platform-specific UI dialogs.
139-142: getInputTextField() is correctly implemented as a synchronous method returning PatrolFinder directly.The
getInputTextField()method inChatGroupDetailRobotis defined asPatrolFinder getInputTextField()withoutasynckeyword and returns$(TextField)directly. This confirms the removal ofawaitin the code snippet is appropriate and consistent with the method implementation.integration_test/robots/profile_information_robot.dart (1)
66-69: LGTM!The navigation helper is straightforward and correctly waits for the target view to be visible before returning.
integration_test/scenarios/chat_detail_scenario.dart (1)
30-40: LGTM!The
parseUserLevelmethod correctly normalizes whitespace, converts to lowercase, and falls back tomemberfor unrecognized values. UsingbyNamewith try-catch is a clean approach.
| try { | ||
| s.softAssertEquals( | ||
| ProfileInformationRobot($).getOnlineStatus().exists, | ||
| true, | ||
| "online status is not shown", | ||
| ); | ||
| } catch (_) { | ||
| // Ignore: Online status element may not exist for some users | ||
| } |
There was a problem hiding this comment.
The try-catch silently discards the assertion result.
Catching and ignoring the exception defeats the purpose of soft assertions. If online status is genuinely optional, use a conditional check instead:
if (ProfileInformationRobot($).getOnlineStatus().exists) {
s.softAssertEquals(
true,
true,
"online status is shown",
);
}Alternatively, remove this block entirely if online status verification isn't required.
🤖 Prompt for AI Agents
integration_test/scenarios/chat_detail_scenario.dart lines 105-113: the
try-catch currently swallows assertion failures for the online-status check;
replace the try-catch with a conditional check that first tests
ProfileInformationRobot($).getOnlineStatus().exists and only calls
s.softAssertEquals when the element exists (or remove the whole block if online
status should not be verified), ensuring you do not silently ignore exceptions
and that assertions run only when the element is present.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
integration_test/tests/chat/chat_group_open_profile_test.dartintegration_test/tests/chat/chat_list_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- integration_test/tests/chat/chat_group_open_profile_test.dart
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-09T06:19:59.615Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2761
File: lib/pages/contacts_tab/widgets/sliver_invite_friend_button.dart:49-57
Timestamp: 2025-12-09T06:19:59.615Z
Learning: For the Twake on Matrix Flutter app (fluffychat), desktop platforms (macOS, Linux, Windows) are not supported. During reviews, assume only web and mobile targets and avoid introducing desktop-specific builds, dependencies, or platform checks. This guideline applies across Dart source files in the repository.
Applied to files:
integration_test/tests/chat/chat_list_test.dart
📚 Learning: 2025-12-22T06:44:09.742Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2644
File: lib/pages/chat/events/audio_message/audio_player_widget.dart:134-134
Timestamp: 2025-12-22T06:44:09.742Z
Learning: When using MatrixFile from matrix-dart-sdk (v4.1.0+), MatrixFile.bytes is non-nullable (Uint8List). Ensure code handles non-null bytes and rely on methods like downloadAndDecryptAttachment() that guarantee non-null returned bytes. Do not assume MatrixFile.bytes may be null; add null-safety checks only where the API guarantees non-null yield, and avoid nullable casts unless upgrading the SDK version. This guideline applies to Dart files across the codebase that interact with MatrixFile.
Applied to files:
integration_test/tests/chat/chat_list_test.dart
⏰ 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). (3)
- GitHub Check: Deploy preview versions on pull requests
- GitHub Check: Analyze code
- GitHub Check: Widget test
| test: ($) async { | ||
| final now = DateTime.now(); | ||
| const groupTest = String.fromEnvironment('TitleOfGroupTest'); | ||
| const groupTest = String.fromEnvironment('SearchByTitle'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n integration_test/tests/chat/chat_list_test.dart | head -150Repository: linagora/twake-on-matrix
Length of output: 6403
🏁 Script executed:
rg -n "fromEnvironment" integration_test/tests/chat/chat_list_test.dartRepository: linagora/twake-on-matrix
Length of output: 409
🏁 Script executed:
rg -n "SearchByTitle\|GroupTest" integration_test/tests/chat/chat_list_test.dartRepository: linagora/twake-on-matrix
Length of output: 50
🏁 Script executed:
rg -n "GroupTest" integration_test/Repository: linagora/twake-on-matrix
Length of output: 748
🏁 Script executed:
rg -n "SearchByTitle" integration_test/Repository: linagora/twake-on-matrix
Length of output: 825
Variable name groupTest is semantically misaligned with its environment variable source SearchByTitle.
The variable groupTest pulls from SearchByTitle, which is used elsewhere in this file (lines 16, 84) for search-related functionality. Consider renaming the variable to searchByTitle (or groupFromSearch) to clarify that this test uses a group obtained via search, not a dedicated test group.
Note: The "Pin/unpin a chat" test at line 130 uses String.fromEnvironment('GroupTest'), which is a different environment variable. Ensure both SearchByTitle and GroupTest are defined in your .env configuration.
🤖 Prompt for AI Agents
In integration_test/tests/chat/chat_list_test.dart around line 102, the variable
name groupTest is misleading because it reads from the SearchByTitle environment
variable; rename it to searchByTitle (or groupFromSearch) to reflect that it
comes from SearchByTitle, update all usages in this file accordingly (including
related search assertions at lines ~16 and ~84), and ensure your .env contains
both SearchByTitle and GroupTest as noted.
… not from the logged-in user.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
integration_test/tests/chat/chat_group_test.dart (1)
79-86: Consider clarifying intent for unusedreceiverMsg.The
receiverMsgis destructured fromprepareTwoMessagesbut is not used in this test. If the receiver message deletion was intentionally removed, consider documenting why or using_to indicate it's intentionally unused:final (senderMsg, _) = await prepareTwoMessages($);This makes it clear the receiver message is needed for test setup but not verified in this specific test.
integration_test/scenarios/chat_scenario.dart (2)
92-92: Typo in parameter name:isMsgOfLogginUsershould beisMsgOfLoggedInUser.The parameter name has a typo that affects code readability and consistency. Consider renaming for clarity.
🔎 Proposed fix
- bool isMsgOfLogginUser = false, + bool isMsgOfLoggedInUser = false,And update all call sites accordingly.
106-108: Consider adding verification that Delete is absent for non-owner messages.The current logic only verifies the Delete item exists when
isMsgOfLogginUseris true, but doesn't verify it's absent otherwise. For complete verification of the PR objective (updating menu items based on message ownership), consider adding an else branch:if (isMsgOfLogginUser) { expect((PullDownMenuRobot($).getDeleteItem()).exists, isTrue); + } else { + expect((PullDownMenuRobot($).getDeleteItem()).exists, isFalse); }This ensures the menu correctly hides the Delete option for messages not owned by the logged-in user.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
integration_test/scenarios/chat_scenario.dartintegration_test/tests/chat/chat_group_test.dart
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-09T05:40:51.607Z
Learnt from: nqhhdev
Repo: linagora/twake-on-matrix PR: 2751
File: lib/pages/chat_draft/draft_chat.dart:608-611
Timestamp: 2025-12-09T05:40:51.607Z
Learning: In draft_chat.dart, when sending media with caption through sendImagesWithCaption, the dialog returns SendMediaWithCaptionStatus.emptyRoom (not done) when the user confirms the send, because no room exists yet. The emptyRoom status triggers room creation and media upload, making it effectively the "success" case. Therefore, text should only be restored when status != emptyRoom.
Applied to files:
integration_test/scenarios/chat_scenario.dart
📚 Learning: 2025-12-09T06:19:59.615Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2761
File: lib/pages/contacts_tab/widgets/sliver_invite_friend_button.dart:49-57
Timestamp: 2025-12-09T06:19:59.615Z
Learning: For the Twake on Matrix Flutter app (fluffychat), desktop platforms (macOS, Linux, Windows) are not supported. During reviews, assume only web and mobile targets and avoid introducing desktop-specific builds, dependencies, or platform checks. This guideline applies across Dart source files in the repository.
Applied to files:
integration_test/scenarios/chat_scenario.dartintegration_test/tests/chat/chat_group_test.dart
📚 Learning: 2025-12-22T06:44:09.742Z
Learnt from: tddang-linagora
Repo: linagora/twake-on-matrix PR: 2644
File: lib/pages/chat/events/audio_message/audio_player_widget.dart:134-134
Timestamp: 2025-12-22T06:44:09.742Z
Learning: When using MatrixFile from matrix-dart-sdk (v4.1.0+), MatrixFile.bytes is non-nullable (Uint8List). Ensure code handles non-null bytes and rely on methods like downloadAndDecryptAttachment() that guarantee non-null returned bytes. Do not assume MatrixFile.bytes may be null; add null-safety checks only where the API guarantees non-null yield, and avoid nullable casts unless upgrading the SDK version. This guideline applies to Dart files across the codebase that interact with MatrixFile.
Applied to files:
integration_test/scenarios/chat_scenario.dartintegration_test/tests/chat/chat_group_test.dart
⏰ 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). (3)
- GitHub Check: Deploy preview versions on pull requests
- GitHub Check: Widget test
- GitHub Check: Analyze code
🔇 Additional comments (9)
integration_test/tests/chat/chat_group_test.dart (2)
6-6: LGTM!The import of
base_scenario.dartcorrectly provides access to theUserLevelenum used in the verification calls below.
49-59: LGTM! Clear separation of sender/receiver message verification.The updated verification logic correctly distinguishes between the logged-in user's message (owner level,
isMsgOfLogginUser: true) and the receiver's message (member level, defaultisMsgOfLogginUser: false). This aligns with the expected behavior where delete option visibility depends on message ownership.integration_test/scenarios/chat_scenario.dart (7)
19-22: LGTM!The new imports correctly support:
TwakeListItemfor the updated chat group selection inopenChatGroupByTitlebase_scenario.dartfor theUserLevelenum used in verification
44-49: LGTM! Improved precision for chat group selection.The updated implementation:
- Uses
TwakeListItem.containing(groupTitle)for precise title matching instead of a generic first-item selection- Waits for the input field to be visible, ensuring the chat detail screen is fully loaded before proceeding
This makes the test more reliable and specific.
119-128: LGTM!Consistent transition to
openPullDownMenuOfAMessagefor message-specific menu interactions.
137-140: LGTM!Consistent update to
openPullDownMenuOfAMessageforcopyMessage.
142-146: LGTM!The synchronous call to
getInputTextField()is consistent with the updated method signature inChatGroupDetailRobot.
181-190: LGTM!Consistent updates to
editMessageandselectMessage.
209-217: LGTM!Consistent updates to
pinMessageandunpinMessage.
| Future<void> deleteMessage(String message) async { | ||
| await ChatGroupDetailRobot($).openPullDownMenu(message); | ||
| await ChatGroupDetailRobot($).openPullDownMenuOfAMessage(message); | ||
| await (PullDownMenuRobot($).getDeleteItem()).tap(); | ||
| await $.native.tap(Selector(text: 'Delete')); | ||
| if (Platform.isIOS) { | ||
| await $.native.tap(Selector(text: 'Delete')); | ||
| } else { | ||
| await $.native.tap( | ||
| Selector( | ||
| className: 'android.widget.Button', | ||
| contentDescription: 'Delete', | ||
| ), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and check the code at lines 219-232
cd integration_test/scenarios && wc -l chat_scenario.dart && echo "---" && sed -n '219,232p' chat_scenario.dartRepository: linagora/twake-on-matrix
Length of output: 525
🏁 Script executed:
# Search for localization/i18n files in the entire codebase
find . -type f \( -name "*.arb" -o -name "*i18n*" -o -name "*l10n*" -o -name "*locale*" -o -name "*translation*" \) 2>/dev/null | head -20Repository: linagora/twake-on-matrix
Length of output: 587
🏁 Script executed:
# Check if there's a pubspec.yaml that might indicate localization setup
fd -e pubspec.yaml | xargs grep -A 5 -B 5 "localization\|i18n\|intl\|flutter_localizations" 2>/dev/null | head -30Repository: linagora/twake-on-matrix
Length of output: 50
🏁 Script executed:
# Search for any hardcoded Delete button references in other test/integration files
rg -i "delete.*selector\|selector.*delete" integration_test/ --type dart -A 2 -B 2 | head -40Repository: linagora/twake-on-matrix
Length of output: 50
🏁 Script executed:
# Check if localization files contain "Delete" string
grep -i "delete" ./assets/l10n/intl_en.arb 2>/dev/null | head -5Repository: linagora/twake-on-matrix
Length of output: 250
🏁 Script executed:
# Check a few other language files for Delete
grep -i "delete" ./assets/l10n/intl_es.arb ./assets/l10n/intl_de.arb ./assets/l10n/intl_ar.arb 2>/dev/null | head -10Repository: linagora/twake-on-matrix
Length of output: 671
🏁 Script executed:
# Check pubspec.yaml for localization configuration
cat pubspec.yaml | head -100Repository: linagora/twake-on-matrix
Length of output: 2694
🏁 Script executed:
# Look for test configuration or setup files
fd -e dart integration_test | xargs grep -l "locale\|Locale" 2>/dev/null | head -5Repository: linagora/twake-on-matrix
Length of output: 50
🏁 Script executed:
# Check if the delete button text is being accessed from localization elsewhere
rg "Delete" --type dart integration_test/ -B 2 -A 2 | head -40Repository: linagora/twake-on-matrix
Length of output: 2489
🏁 Script executed:
# Search for test setup files and locale configuration
fd -e dart integration_test/ | xargs grep -l "setUpAll\|setUp\|locale\|Locale" 2>/dev/nullRepository: linagora/twake-on-matrix
Length of output: 443
🏁 Script executed:
# Check test main file or integration test configuration
fd integration_test.dart -o *_test.dart integration_test/ | head -5 | xargs cat 2>/dev/null | head -80Repository: linagora/twake-on-matrix
Length of output: 209
🏁 Script executed:
# Check if there's documentation about test locale
fd -e md -e txt | xargs grep -i "integration.*test\|test.*locale" 2>/dev/null | head -10Repository: linagora/twake-on-matrix
Length of output: 848
🏁 Script executed:
# Examine the menu_robot.dart to see the getDeleteItem implementation in full context
sed -n '1,50p' integration_test/robots/menu_robot.dart && echo "..." && grep -A 5 -B 5 "getDeleteItem" integration_test/robots/menu_robot.dartRepository: linagora/twake-on-matrix
Length of output: 1641
Integration test will fail in non-English locales due to hardcoded English text.
The app supports 20+ languages, but this test uses hardcoded 'Delete' strings that won't match localized UI elements. When the app runs in Spanish, for example, the menu item displays "Eliminar" instead of "Delete", causing the test to fail.
Additionally, menu_robot.dart's getDeleteItem() method uses find.text("Delete"), which is also locale-dependent. Both the menu finder and the native selectors need to handle localized strings or enforce a consistent test locale.
🤖 Prompt for AI Agents
integration_test/scenarios/chat_scenario.dart lines 219-232: the test is failing
in non-English locales because the code taps UI elements by the hardcoded
English string "Delete" (both in this file's native Selector taps and in
menu_robot.dart's getDeleteItem() which uses find.text("Delete")); either modify
the test to use a locale-independent finder (preferably change getDeleteItem()
to find.byKey or find.bySemanticsLabel using a stable testKey/semanticsLabel on
the delete menu item and use that here instead of text, and update native
selectors to target the same accessibility identifier/contentDescription or
resource id), or explicitly set the app/test locale to English at test setup
before these interactions; update both places (menu_robot.dart and this native
tap branch) to use the chosen locale-independent identifier so the test passes
across locales.
2 participants
Ticket
this ticket update 7 scripts relate to react on item of PullDownMenu
Resolved
https://github.com/user-attachments/assets/1e21b4ff-c425-4524-909f-e5b41354c1f3
https://github.com/user-attachments/assets/ac9f69c0-731f-40ae-8d0f-3078feac3d49
https://github.com/user-attachments/assets/31bd1629-1562-4e32-8ba2-500cb4000d68
https://github.com/user-attachments/assets/f50ce008-2a40-459a-bdd0-412447813e70
https://github.com/user-attachments/assets/c04cc072-d7ff-4091-8270-44d54cd1af64
https://github.com/user-attachments/assets/0150580e-074c-44dd-95f8-fd2f60dafde7
https://github.com/user-attachments/assets/57fdb3f9-db05-4ba4-a618-d5d4ada87629
video when run on Android:
-case "see message info"
https://github.com/user-attachments/assets/5d40ea51-e4a9-4d1c-9a47-159d25df20b4
-case "edit message"
https://github.com/user-attachments/assets/f38b3ce5-5103-4587-b251-f46e4fb6617c
-Case " copy message"
https://github.com/user-attachments/assets/752d2b54-9e1d-40a6-9f57-66e1f05b521b
-Case "delete a message"
https://github.com/user-attachments/assets/5c23d5ad-2f3f-4720-b8e8-e441599a66dc
-Case reply message
https://github.com/user-attachments/assets/762c1a21-8f71-4fee-b3b4-6db225ba1fc1
-Case "verify items of pull down menu when right click on a message"
https://github.com/user-attachments/assets/a9078e8f-7d5f-4ef0-b224-486611958fda
Summary by CodeRabbit
Tests
Chores
Stability
✏️ Tip: You can customize this high-level summary in your review settings.