-
Notifications
You must be signed in to change notification settings - Fork 148
fix: install dependencies so analysis options work during format #269
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
WalkthroughCI workflow was modified to run on the Ubuntu hosted runner (container removed), install Flutter via subosito/flutter-action@v2, run ✨ 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 (
|
Because the flutter_lints dev dependency wasn't installed, dart format couldn't resolve the analysis_options.yaml properly and didn't fully format everything correctly.
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/format.yml (1)
1-9: Grant write permission for auto-commit in.github/workflows/format.ymlAdd an explicit permissions block to allow the GITHUB_TOKEN to push formatting changes (without this, the Add & Commit step will fail):
name: Format and push +permissions: + contents: writeOptionally, guard the commit step to skip forked PRs:
- name: Add & Commit uses: EndBug/add-and-commit@v9.1.4 + if: ${{ github.event.pull_request.head.repo.fork == false }} with: add: lib
🧹 Nitpick comments (5)
.github/workflows/format.yml (5)
14-14: Fix YAML indentation under container.Current indentation violates yamllint; use 6 spaces under
container:instead of 8.- image: dart:stable + image: dart:stable
17-22: Harden checkout for PR branches and pushes.To reliably push commits back to the PR branch (especially from forks) fetch full history and target the source repo explicitly.
- name: Checkout repository uses: actions/checkout@v4 with: persist-credentials: true - ref: ${{ github.event.pull_request.head.ref }} + fetch-depth: 0 + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.ref }}
26-28: Prefer check mode to mutation in CI (optional).If you don’t want CI to write commits, use a failing check:
- - name: Format Dart code - run: dart format . + - name: Format Dart code (check only) + run: dart format --output=none --set-exit-if-changed .
29-31: Make safe.directory portable.Use
$GITHUB_WORKSPACEso the path doesn’t break if repo/owner changes.- - name: git config - run: git config --global --add safe.directory /__w/sdk-for-flutter/sdk-for-flutter # required to fix dubious ownership + - name: git config + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" # required to fix dubious ownership
32-36: Scope of auto-commit may miss formatted files.
dart format .touches more thanlib/. If intentional, keep as-is; otherwise widen the add pattern.- add: lib + add: .Optional: also set a clear commit message/author to avoid noisy histories:
message: "chore(format): apply dart format" default_author: github_actions
📜 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 by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/format.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/format.yml
[warning] 14-14: wrong indentation: expected 6 but found 8
(indentation)
⏰ 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: test
283e304 to
05c258d
Compare
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: 10
🧹 Nitpick comments (32)
test/src/models/identity_test.dart (1)
30-32: Avoid secret-scanner false positives in test fixtures.
Gitleaksis flagging the access/refresh token literals as generic API keys. Since these are dummy values, prefer obviously fake placeholders to prevent noisy scans (or add a repo-level allowlist entry for this file/pattern).Apply this diff for the changed lines:
- expect(result.providerAccessToken, 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3'); + expect(result.providerAccessToken, 'dummy_access_token'); - expect(result.providerAccessTokenExpiry, '2020-10-15T06:38:00.000+00:00'); + expect(result.providerAccessTokenExpiry, '2020-10-15T06:38:00.000+00:00'); - expect(result.providerRefreshToken, 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3'); + expect(result.providerRefreshToken, 'dummy_refresh_token');And update the model construction accordingly (outside the changed hunk):
final model = Identity( // ... providerAccessToken: 'dummy_access_token', providerAccessTokenExpiry: '2020-10-15T06:38:00.000+00:00', providerRefreshToken: 'dummy_refresh_token', );If you must keep the realistic-looking tokens, consider adding an allowlist entry in
.gitleaksignorescoped totest/src/models/identity_test.dartwith a tight regex.test/src/models/session_test.dart (1)
49-51: Avoid secret-scan false positives: use obviously fake tokens.Gitleaks flagged these values as “Generic API Key.” Swap to clearly dummy strings.
- expect(result.providerAccessToken, 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3'); + expect(result.providerAccessToken, 'test_access_token'); ... - expect(result.providerRefreshToken, 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3'); + expect(result.providerRefreshToken, 'test_refresh_token');Also update the constructor above accordingly:
providerAccessToken: 'test_access_token', providerRefreshToken: 'test_refresh_token',test/src/models/jwt_test.dart (1)
8-9: Use a non-token-looking placeholder to silence secret scanners.The hardcoded JWT trips Gitleaks. A simple “x.y.z” keeps intent without false positives.
- final model = Jwt( - jwt: - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c', + final model = Jwt( + jwt: 'x.y.z', ); ... - expect(result.jwt, - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'); + expect(result.jwt, 'x.y.z');Also applies to: 15-17
test/services/databases_test.dart (2)
66-69: Strengthen Mockito stubs: match named args explicitly.Current stubs rely on defaults; explicit named matchers make intent clear and prevent brittle matches if defaults change.
- when(client.call( - HttpMethod.get, - )).thenAnswer((_) async => Response(data: data)); + when(client.call( + HttpMethod.get, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).thenAnswer((_) async => Response(data: data));Apply similarly to post/put/patch/delete stubs in this file.
Also applies to: 88-91, 112-115, 135-138, 159-162, 174-177, 196-199, 220-223
171-183: Add a verify/assertion for deleteDocument.Ensure the call is made and with the expected method at least once.
final response = await databases.deleteDocument( databaseId: '<DATABASE_ID>', collectionId: '<COLLECTION_ID>', documentId: '<DOCUMENT_ID>', ); + verify(client.call( + HttpMethod.delete, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).called(1); + expect(response, isA<void>());.github/workflows/format.yml (4)
20-20: Trim trailing whitespace.YAMLlint flags trailing spaces on this blank line.
38-38: Fix indentation under with:.Indentation is off (expected 10, found 12). Align the add key with other with-entries.
- add: '["lib", "test"]' + add: '["lib", "test"]'
21-28: Cache Flutter to speed up workflow.Enable action caching to cut CI time.
- name: Install Flutter uses: subosito/flutter-action@v2 with: channel: stable + cache: true
35-39: Guard commits from forked PRs.Prevent failures when the workflow runs on forks without push rights.
- - name: Add & Commit + - name: Add & Commit + if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} uses: EndBug/add-and-commit@v9.1.4 with: add: '["lib", "test"]'test/services/locale_test.dart (2)
71-74: Use explicit named argument matchers in stubs.Same reasoning as databases_test.dart—clearer stubbing and less brittle tests.
- when(client.call( - HttpMethod.get, - )).thenAnswer((_) async => Response(data: data)); + when(client.call( + HttpMethod.get, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).thenAnswer((_) async => Response(data: data));Also applies to: 85-88, 99-102, 113-116, 127-130, 141-144, 155-158, 169-172
60-77: Optionally verify the HTTP method per call.Add verify(...) after calls to assert the expected method is used.
test/services/tables_db_test.dart (3)
170-182: Assert delete completes and remove unused variableAvoid unused local variable and assert behavior with
completes.Apply this diff:
- final data = ''; - - when(client.call( - HttpMethod.delete, - )).thenAnswer((_) async => Response(data: data)); - - final response = await tablesDB.deleteRow( - databaseId: '<DATABASE_ID>', - tableId: '<TABLE_ID>', - rowId: '<ROW_ID>', - ); + when(client.call( + HttpMethod.delete, + )).thenAnswer((_) async => Response(data: '')); + + await expectLater( + tablesDB.deleteRow( + databaseId: '<DATABASE_ID>', + tableId: '<TABLE_ID>', + rowId: '<ROW_ID>', + ), + completes, + );
4-4: Remove unused imports to satisfy flutter_lintsThese aren’t used in this test file.
Apply this diff:
-import 'package:appwrite/enums.dart' as enums; -import 'dart:typed_data';Also applies to: 7-7
60-75: Consider verifying request path/params in addition to return typesRight now tests only assert types. Add
verify(...)withanyNamedforpath,headers, andparamsto ensure the SDK uses expected endpoints.Example:
verify(client.call( HttpMethod.get, path: anyNamed('path'), headers: anyNamed('headers'), params: anyNamed('params'), )).called(1);Also applies to: 77-99, 101-122, 124-145, 147-168, 184-206, 208-230
test/services/storage_test.dart (3)
159-170: Assert delete completes and drop unused localUse
completesto make the expectation explicit.Apply this diff:
- final data = ''; - - when(client.call( - HttpMethod.delete, - )).thenAnswer((_) async => Response(data: data)); - - final response = await storage.deleteFile( - bucketId: '<BUCKET_ID>', - fileId: '<FILE_ID>', - ); + when(client.call( + HttpMethod.delete, + )).thenAnswer((_) async => Response(data: '')); + + await expectLater( + storage.deleteFile( + bucketId: '<BUCKET_ID>', + fileId: '<FILE_ID>', + ), + completes, + );
4-4: Remove unused import
enumsalias isn’t used in this file.Apply this diff:
-import 'package:appwrite/enums.dart' as enums;
60-74: Optional: verify method/path/params for robustnessAdd
verify(...)calls to ensureStoragehits expected endpoints, not just returns types.Example:
verify(client.call( anyOf(HttpMethod.get, HttpMethod.post, HttpMethod.put), path: anyNamed('path'), headers: anyNamed('headers'), params: anyNamed('params'), )).called(1);Also applies to: 76-105, 107-131, 133-157, 172-184, 186-198, 200-212
test/services/avatars_test.dart (2)
3-3: Remove unused models importNot referenced in this file.
Apply this diff:
-import 'package:appwrite/models.dart' as models;
60-71: Optional: verify request constructionAdd
verify(...)to assertHttpMethod.getand non-emptypathfor each call.Example:
verify(client.call( HttpMethod.get, path: anyNamed('path'), params: anyNamed('params'), headers: anyNamed('headers'), )).called(1);Also applies to: 73-84, 86-97, 99-110, 112-123, 125-134, 136-147
test/services/functions_test.dart (2)
4-4: Remove unused imports
enumsanddart:typed_dataaren’t used.Apply this diff:
-import 'package:appwrite/enums.dart' as enums; -import 'dart:typed_data';Also applies to: 7-7
60-74: Optional: add verify for method/pathEnsure Functions hits expected endpoints and uses correct verb.
Example:
verify(client.call( anyOf(HttpMethod.get, HttpMethod.post), path: anyNamed('path'), headers: anyNamed('headers'), params: anyNamed('params'), )).called(1);Also applies to: 76-105, 107-137
test/services/teams_test.dart (4)
136-146: Teams.delete: assert completion and remove unused variableSwitch to
completes.Apply this diff:
- final data = ''; - - when(client.call( - HttpMethod.delete, - )).thenAnswer((_) async => Response(data: data)); - - final response = await teams.delete( - teamId: '<TEAM_ID>', - ); + when(client.call( + HttpMethod.delete, + )).thenAnswer((_) async => Response(data: '')); + + await expectLater( + teams.delete( + teamId: '<TEAM_ID>', + ), + completes, + );
249-260: Teams.deleteMembership: assert completion and drop unused variableSame pattern as above.
Apply this diff:
- final data = ''; - - when(client.call( - HttpMethod.delete, - )).thenAnswer((_) async => Response(data: data)); - - final response = await teams.deleteMembership( - teamId: '<TEAM_ID>', - membershipId: '<MEMBERSHIP_ID>', - ); + when(client.call( + HttpMethod.delete, + )).thenAnswer((_) async => Response(data: '')); + + await expectLater( + teams.deleteMembership( + teamId: '<TEAM_ID>', + membershipId: '<MEMBERSHIP_ID>', + ), + completes, + );
4-4: Remove unused imports
enumsanddart:typed_dataare unused here.Apply this diff:
-import 'package:appwrite/enums.dart' as enums; -import 'dart:typed_data';Also applies to: 7-7
60-72: Optional: verify method/path across operationsAdd
verify(...)checks withanyNamedto ensure correct verbs and non-empty paths across teams and membership endpoints.Example:
verify(client.call( anyOf(HttpMethod.get, HttpMethod.post, HttpMethod.put, HttpMethod.patch, HttpMethod.delete), path: anyNamed('path'), headers: anyNamed('headers'), params: anyNamed('params'), )).called(1);Also applies to: 74-93, 95-113, 115-134, 148-162, 164-190, 192-218, 220-247, 262-290, 292-303, 305-317
test/services/account_test.dart (7)
27-31: Capture named args in webAuth Invocation for accurate stubbing/verificationYou're dropping
callbackUrlSchemein thenoSuchMethodInvocation, which makes futureverify/matching brittle. Include named args.@override Future webAuth( Uri? url, { String? callbackUrlScheme, }) async { - return super - .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); + return super.noSuchMethod( + Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), + returnValue: 'done', + ); }
43-47: Also capture named args for chunkedUpload InvocationPositional-packing named parameters loses information and prevents precise stubbing/verification.
- return super.noSuchMethod( - Invocation.method( - #chunkedUpload, [path, params, paramName, idParamName, headers]), - returnValue: Response(data: {})); + return super.noSuchMethod( + Invocation.method(#chunkedUpload, [], { + #path: path, + #params: params, + #paramName: paramName, + #idParamName: idParamName, + #headers: headers, + #onProgress: onProgress, + }), + returnValue: Response(data: {}), + );
80-83: Broaden mock stubs to include named args (applies throughout file)Current
when(client.call(HttpMethod.x,))ignores named args. Prefer matching them to avoid brittle stubs as APIs evolve.Example refactor (apply similarly to other stubs):
- when(client.call( - HttpMethod.get, - )).thenAnswer((_) async => Response(data: data)); + when(client.call( + HttpMethod.get, + path: anyNamed('path'), + headers: anyNamed('headers'), + params: anyNamed('params'), + responseType: anyNamed('responseType'), + )).thenAnswer((_) async => Response(data: data));Also applies to: 108-111, 140-143
915-919: Use matchers for webAuth stubs; avoid hardcodingUri()This makes the stub resilient to URL changes and allows verifying the callback scheme if needed.
- when(client.webAuth( - Uri(), - )).thenAnswer((_) async => 'done'); + when(client.webAuth( + any, + callbackUrlScheme: anyNamed('callbackUrlScheme'), + )).thenAnswer((_) async => 'done');Also applies to: 1240-1243
172-175: Drop unusedresponsevars in void/empty-return testsPrevents
unused_local_variablewarnings and keeps tests lean.Example:
- final response = await account.deleteIdentity( + await account.deleteIdentity( identityId: '<IDENTITY_ID>', );Apply similarly to other delete/void calls in this file.
Also applies to: 336-339, 348-351, 783-784, 1106-1109, 1192-1195
235-249: Deduplicate MFA alias tests or parameterizeYou test both
MfaandMFAvariants. If both are intentional aliases, consider a parameterized test to reduce duplication and runtime; otherwise, keep one and add a single alias test for coverage.Also applies to: 251-266
60-78: Extract common fixtures for repeated mapsLarge repeated user/session maps can be factored into helpers to cut noise and focus assertions on behavior.
Also applies to: 88-106, 205-233, 587-616
📜 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 by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (70)
.github/workflows/format.yml(2 hunks)lib/query.dart(1 hunks)lib/services/account.dart(0 hunks)lib/services/avatars.dart(0 hunks)lib/services/storage.dart(0 hunks)lib/src/client.dart(1 hunks)lib/src/cookie_manager.dart(1 hunks)lib/src/realtime_io.dart(1 hunks)lib/src/realtime_mixin.dart(1 hunks)lib/src/realtime_stub.dart(1 hunks)test/query_test.dart(0 hunks)test/services/account_test.dart(2 hunks)test/services/avatars_test.dart(2 hunks)test/services/databases_test.dart(2 hunks)test/services/functions_test.dart(2 hunks)test/services/graphql_test.dart(2 hunks)test/services/locale_test.dart(2 hunks)test/services/messaging_test.dart(2 hunks)test/services/storage_test.dart(2 hunks)test/services/tables_db_test.dart(2 hunks)test/services/teams_test.dart(2 hunks)test/src/models/algo_argon2_test.dart(1 hunks)test/src/models/algo_bcrypt_test.dart(1 hunks)test/src/models/algo_md5_test.dart(1 hunks)test/src/models/algo_phpass_test.dart(1 hunks)test/src/models/algo_scrypt_modified_test.dart(1 hunks)test/src/models/algo_scrypt_test.dart(1 hunks)test/src/models/algo_sha_test.dart(1 hunks)test/src/models/continent_list_test.dart(1 hunks)test/src/models/continent_test.dart(1 hunks)test/src/models/country_list_test.dart(1 hunks)test/src/models/country_test.dart(1 hunks)test/src/models/currency_list_test.dart(1 hunks)test/src/models/currency_test.dart(1 hunks)test/src/models/document_list_test.dart(1 hunks)test/src/models/document_test.dart(1 hunks)test/src/models/execution_list_test.dart(1 hunks)test/src/models/execution_test.dart(1 hunks)test/src/models/file_list_test.dart(1 hunks)test/src/models/file_test.dart(1 hunks)test/src/models/headers_test.dart(1 hunks)test/src/models/identity_list_test.dart(1 hunks)test/src/models/identity_test.dart(1 hunks)test/src/models/jwt_test.dart(1 hunks)test/src/models/language_list_test.dart(1 hunks)test/src/models/language_test.dart(1 hunks)test/src/models/locale_code_list_test.dart(1 hunks)test/src/models/locale_code_test.dart(1 hunks)test/src/models/locale_test.dart(1 hunks)test/src/models/log_list_test.dart(1 hunks)test/src/models/log_test.dart(1 hunks)test/src/models/membership_list_test.dart(1 hunks)test/src/models/membership_test.dart(1 hunks)test/src/models/mfa_challenge_test.dart(1 hunks)test/src/models/mfa_factors_test.dart(1 hunks)test/src/models/mfa_recovery_codes_test.dart(1 hunks)test/src/models/mfa_type_test.dart(1 hunks)test/src/models/phone_list_test.dart(1 hunks)test/src/models/phone_test.dart(1 hunks)test/src/models/preferences_test.dart(0 hunks)test/src/models/row_list_test.dart(1 hunks)test/src/models/row_test.dart(1 hunks)test/src/models/session_list_test.dart(1 hunks)test/src/models/session_test.dart(1 hunks)test/src/models/subscriber_test.dart(2 hunks)test/src/models/target_test.dart(1 hunks)test/src/models/team_list_test.dart(1 hunks)test/src/models/team_test.dart(1 hunks)test/src/models/token_test.dart(1 hunks)test/src/models/user_test.dart(1 hunks)
💤 Files with no reviewable changes (5)
- lib/services/account.dart
- lib/services/storage.dart
- test/query_test.dart
- lib/services/avatars.dart
- test/src/models/preferences_test.dart
✅ Files skipped from review due to trivial changes (53)
- test/src/models/country_test.dart
- test/src/models/locale_test.dart
- test/src/models/country_list_test.dart
- test/src/models/currency_list_test.dart
- test/src/models/headers_test.dart
- test/src/models/session_list_test.dart
- test/src/models/mfa_type_test.dart
- test/src/models/locale_code_list_test.dart
- test/src/models/continent_list_test.dart
- test/src/models/locale_code_test.dart
- lib/src/cookie_manager.dart
- lib/src/realtime_mixin.dart
- lib/query.dart
- test/src/models/membership_test.dart
- test/src/models/identity_list_test.dart
- test/src/models/algo_md5_test.dart
- lib/src/realtime_stub.dart
- test/src/models/document_test.dart
- test/src/models/language_list_test.dart
- test/src/models/algo_phpass_test.dart
- test/src/models/language_test.dart
- test/src/models/execution_test.dart
- lib/src/realtime_io.dart
- test/src/models/mfa_recovery_codes_test.dart
- test/src/models/phone_list_test.dart
- test/src/models/team_list_test.dart
- test/src/models/mfa_factors_test.dart
- test/src/models/user_test.dart
- test/src/models/continent_test.dart
- test/src/models/row_test.dart
- test/src/models/log_list_test.dart
- test/src/models/currency_test.dart
- test/src/models/document_list_test.dart
- test/src/models/token_test.dart
- test/src/models/algo_argon2_test.dart
- test/src/models/team_test.dart
- test/src/models/log_test.dart
- test/src/models/algo_bcrypt_test.dart
- test/src/models/target_test.dart
- test/src/models/execution_list_test.dart
- test/src/models/mfa_challenge_test.dart
- test/src/models/file_test.dart
- test/src/models/file_list_test.dart
- test/src/models/subscriber_test.dart
- lib/src/client.dart
- test/src/models/algo_scrypt_modified_test.dart
- test/src/models/membership_list_test.dart
- test/src/models/row_list_test.dart
- test/src/models/algo_sha_test.dart
- test/src/models/algo_scrypt_test.dart
- test/services/graphql_test.dart
- test/services/messaging_test.dart
- test/src/models/phone_test.dart
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/format.yml
[error] 20-20: trailing spaces
(trailing-spaces)
[warning] 38-38: wrong indentation: expected 10 but found 12
(indentation)
🪛 Gitleaks (8.27.2)
test/src/models/identity_test.dart
30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/src/models/jwt_test.dart
15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/src/models/session_test.dart
49-49: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (10)
test/src/models/identity_test.dart (1)
23-33: Formatting-only change — LGTM.Assertions remain identical and validate round-trip from
toMap/fromMap. No functional changes observed.test/src/models/session_test.dart (1)
42-71: LGTM: formatting-only; assertions unchanged.test/services/databases_test.dart (1)
51-59: Good refactor: shared setUp improves readability and reduces duplication.test/services/locale_test.dart (1)
51-59: Nice: centralized setUp improves consistency across tests.test/services/tables_db_test.dart (1)
51-58: Good move to shared setUpShared fixture reduces duplication and improves readability.
test/services/storage_test.dart (1)
51-58: Shared setUp LGTMCleaner and consistent with the rest of the suite.
test/services/avatars_test.dart (1)
51-58: setUp refactor looks goodConsistent initialization across tests.
test/services/functions_test.dart (1)
51-58: Shared setUp: nice standardizationReduces repetition.
test/services/teams_test.dart (1)
51-58: setUp refactor LGTMImproves clarity and consistency across tests.
test/services/account_test.dart (1)
51-58: LGTM: shared setUp improves test readability and reduces duplicationThe per-group
setUpforMockClient/Accountis a solid cleanup.
| Future webAuth( | ||
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
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
MockClient.webAuth: forward named args
Aligns Invocation with method signature.
Apply this diff:
@override
Future webAuth(
Uri? url, {
String? callbackUrlScheme,
}) async {
- return super
- .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
}🤖 Prompt for AI Agents
In test/services/avatars_test.dart around lines 26 to 32, the mock webAuth
invocation currently only forwards the positional arg and omits the named
argument, so update the noSuchMethod call to include the namedArguments map
matching the method signature (use the symbol for the named parameter key) so
Invocation.method mirrors webAuth(url, callbackUrlScheme: ...); keep the same
returnValue.
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); |
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
MockClient.chunkedUpload: include named args in Invocation
Prevents mockito matcher issues for named parameters.
Apply this diff:
- return super.noSuchMethod(
- Invocation.method(
- #chunkedUpload, [path, params, paramName, idParamName, headers]),
- returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );📝 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.
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| return super.noSuchMethod( | |
| Invocation.method(#chunkedUpload, const [], { | |
| #path: path, | |
| #params: params, | |
| #paramName: paramName, | |
| #idParamName: idParamName, | |
| #headers: headers, | |
| #onProgress: onProgress, | |
| }), | |
| returnValue: Response(data: {}), | |
| ); |
🤖 Prompt for AI Agents
In test/services/avatars_test.dart around lines 43 to 46, the mock Invocation
for chunkedUpload only passes positional arguments which causes mockito matcher
issues for named parameters; update the Invocation.method call to include a
namedArguments map as the third parameter, using Symbol keys (e.g. #paramName,
#idParamName, #headers) mapped to the corresponding local variables so the
invocation signature matches the real method's named parameters.
| Future webAuth( | ||
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
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
MockClient.webAuth: propagate named args
Match on callbackUrlScheme.
Apply this diff:
@override
Future webAuth(
Uri? url, {
String? callbackUrlScheme,
}) async {
- return super
- .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
}📝 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.
| Future webAuth( | |
| Uri? url, | |
| { | |
| String? callbackUrlScheme, | |
| } | |
| ) async { | |
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| } | |
| @override | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), | |
| returnValue: 'done', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In test/services/functions_test.dart around lines 26 to 32, the mock webAuth
method currently calls noSuchMethod without propagating the named argument
callbackUrlScheme so interactions that match on that name will fail; update the
noSuchMethod invocation to include the namedArguments map with
Symbol('callbackUrlScheme') mapped to callbackUrlScheme (e.g.
Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme})) so
the mock matches calls that pass the named parameter and returns the expected
value.
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); |
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
MockClient.chunkedUpload: use named args Invocation
Future-proof stubbing/verification.
Apply this diff:
- return super.noSuchMethod(
- Invocation.method(
- #chunkedUpload, [path, params, paramName, idParamName, headers]),
- returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );📝 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.
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, | |
| const [], | |
| { | |
| #path: path, | |
| #params: params, | |
| #paramName: paramName, | |
| #idParamName: idParamName, | |
| #headers: headers, | |
| #onProgress: onProgress, | |
| }, | |
| ), | |
| returnValue: Response(data: {}), | |
| ); |
🤖 Prompt for AI Agents
In test/services/functions_test.dart around lines 43 to 46, the
Invocation.method currently passes all arguments positionally which prevents
future-proof stubbing/verification; change the call to use Invocation.method
with the same positional list (if any) and a namedArguments map for the named
parameters — e.g. move any parameters that are named in the real chunkedUpload
signature into a fourth argument to Invocation.method as a map of Symbol keys
(use #paramName) to the corresponding local variables (e.g., {#params: params,
#paramName: paramName, #idParamName: idParamName, #headers: headers}), and keep
returnValue: Response(data: {}) the same.
| Future webAuth( | ||
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
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
MockClient.webAuth: forward named args in Invocation
Ensure Mockito stubs using named args match.
Apply this diff:
@override
Future webAuth(
Uri? url, {
String? callbackUrlScheme,
}) async {
- return super
- .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
}🤖 Prompt for AI Agents
In test/services/storage_test.dart around lines 26 to 32, the mock webAuth
method forwards only the positional arg in the Invocation, which breaks Mockito
stubs that rely on the named callbackUrlScheme; update the noSuchMethod call to
include the named argument map so the invocation mirrors the real signature
(pass [url] as positional args and a named args map with
Symbol('callbackUrlScheme') or #callbackUrlScheme mapped to callbackUrlScheme),
and keep the same returnValue.
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); |
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
MockClient.chunkedUpload: use named args in Invocation (+ onProgress)
Prevents stub mismatch on named params in when(client.chunkedUpload(...)).
Apply this diff:
- return super.noSuchMethod(
- Invocation.method(
- #chunkedUpload, [path, params, paramName, idParamName, headers]),
- returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );📝 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.
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, | |
| const [], | |
| { | |
| #path: path, | |
| #params: params, | |
| #paramName: paramName, | |
| #idParamName: idParamName, | |
| #headers: headers, | |
| #onProgress: onProgress, | |
| }, | |
| ), | |
| returnValue: Response(data: {}), | |
| ); |
🤖 Prompt for AI Agents
In test/services/storage_test.dart around lines 43 to 46, the mock
implementation of chunkedUpload uses a positional Invocation which causes stub
mismatches for named parameters; update the Invocation to Invocation.method with
a namedArguments map (using Symbol(...) keys for each named param: #path,
#params, #paramName, #idParamName, #headers and include the onProgress
parameter) and pass the same returnValue: Response(data: {}); this ensures the
mock matches calls that use named args including onProgress.
| Future webAuth( | ||
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
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
MockClient.webAuth: include named arguments in Invocation for Mockito to match stubs
Without passing #callbackUrlScheme in Invocation.method, stubs/verify that rely on named args may not match reliably.
Apply this diff:
@override
Future webAuth(
Uri? url, {
String? callbackUrlScheme,
}) async {
- return super
- .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
}📝 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.
| Future webAuth( | |
| Uri? url, | |
| { | |
| String? callbackUrlScheme, | |
| } | |
| ) async { | |
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| } | |
| @override | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| #webAuth, | |
| [url], | |
| {#callbackUrlScheme: callbackUrlScheme}, | |
| ), | |
| returnValue: 'done', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In test/services/tables_db_test.dart around lines 26 to 32, the mock webAuth
method currently calls super.noSuchMethod with Invocation.method(#webAuth,
[url]) which omits the named argument callbackUrlScheme so Mockito
stubs/verifications using named args won't match; update the Invocation to
include the named arguments map (e.g., Invocation.method(#webAuth, [url],
{#callbackUrlScheme: callbackUrlScheme})) and keep the same returnValue to
ensure stubs and verify calls that pass callbackUrlScheme will correctly match.
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); |
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
❓ Verification inconclusive
MockClient.chunkedUpload: pass named args in Invocation (and include onProgress)
Current override builds an Invocation with positional args only; stubs using named args won’t match. Also forwards onProgress.
Apply this diff:
- return super.noSuchMethod(
- Invocation.method(
- #chunkedUpload, [path, params, paramName, idParamName, headers]),
- returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );Also consider applying the same pattern to call(...):
return super.noSuchMethod(
Invocation.method(#call, [method], {
#path: path,
#headers: headers,
#params: params,
#responseType: responseType,
}),
returnValue: Response(),
);Use named args (including #onProgress) in Invocation.method for chunkedUpload override
Override currently uses only positional args; stubbed calls with named parameters won’t match. Apply this diff in test/services/tables_db_test.dart:
- return super.noSuchMethod(
- Invocation.method(
- #chunkedUpload, [path, params, paramName, idParamName, headers]),
- returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );Also update the call(...) override similarly to use named args.
🤖 Prompt for AI Agents
In test/services/tables_db_test.dart around lines 43 to 46, the mock override
for chunkedUpload currently uses only positional arguments so invocations that
include named parameters (like onProgress) won't match; change the
Invocation.method call to pass a namedArguments map (e.g., {#onProgress:
onProgress}) along with positional args, and do the same update for the
call(...) override elsewhere in the file so both stubs use Invocation.method
with the correct namedArguments Symbols for all named params.
| Future webAuth( | ||
| Uri? url, | ||
| { | ||
| String? callbackUrlScheme, | ||
| } | ||
| ) async { | ||
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } |
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
MockClient.webAuth: include named args
Forward callbackUrlScheme in Invocation.
Apply this diff:
@override
Future webAuth(
Uri? url, {
String? callbackUrlScheme,
}) async {
- return super
- .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done');
+ return super.noSuchMethod(
+ Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}),
+ returnValue: 'done',
+ );
}📝 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.
| Future webAuth( | |
| Uri? url, | |
| { | |
| String? callbackUrlScheme, | |
| } | |
| ) async { | |
| return super.noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| } | |
| @override | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method(#webAuth, [url], {#callbackUrlScheme: callbackUrlScheme}), | |
| returnValue: 'done', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In test/services/teams_test.dart around lines 26 to 32, the mock implementation
of webAuth calls super.noSuchMethod with only a positional Invocation and omits
the named argument callbackUrlScheme; update the call to forward the named
argument by using Invocation.method(#webAuth, [url], {#callbackUrlScheme:
callbackUrlScheme}) so the mock records and matches the named parameter
correctly and returns the same 'done' returnValue.
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); |
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
MockClient.chunkedUpload: switch to named args Invocation
Keeps stubbing/verification reliable.
Apply this diff:
- return super.noSuchMethod(
- Invocation.method(
- #chunkedUpload, [path, params, paramName, idParamName, headers]),
- returnValue: Response(data: {}));
+ return super.noSuchMethod(
+ Invocation.method(#chunkedUpload, const [], {
+ #path: path,
+ #params: params,
+ #paramName: paramName,
+ #idParamName: idParamName,
+ #headers: headers,
+ #onProgress: onProgress,
+ }),
+ returnValue: Response(data: {}),
+ );📝 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.
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| return super.noSuchMethod( | |
| Invocation.method(#chunkedUpload, const [], { | |
| #path: path, | |
| #params: params, | |
| #paramName: paramName, | |
| #idParamName: idParamName, | |
| #headers: headers, | |
| #onProgress: onProgress, | |
| }), | |
| returnValue: Response(data: {}), | |
| ); |
🤖 Prompt for AI Agents
In test/services/teams_test.dart around lines 43-46, the mocked Invocation
currently passes all arguments positionally; change it to supply the
optional/named arguments via the Invocation.namedArguments map so
stubbing/verification is reliable. Replace the Invocation.method(...) call to
include a third parameter: a Map<Symbol, dynamic> with entries for the named
parameters (e.g. {#params: params, #paramName: paramName, #idParamName:
idParamName, #headers: headers}) while keeping the positional list with just the
required positional arg(s) (e.g. [path]), and return the same Response(data: {})
as the returnValue.
What does this PR do?
Because the flutter_lints dev dependency wasn't installed, dart format couldn't resolve the analysis_options.yaml properly and didn't fully format everything correctly.
Test Plan
None
Related PRs and Issues
None
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit
Chores
Tests
Style