Comprehensive upgrade of Kinde Flutter SDK test coverage#53
Comprehensive upgrade of Kinde Flutter SDK test coverage#53KeeganBeuthin wants to merge 7 commits intokinde-oss:mainfrom
Conversation
… test platform channel issue remedied
WalkthroughAdds a large suite of unit tests and test helpers across the SDK covering parameter serialization, auth config, auth state, token API/utils, web flows, error mapping, refresh interceptor, store behavior, URL helpers, in-memory mocks, and JWT/fixture generators. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (6)
test/kinde_flutter_sdk_core_test.dart (1)
96-106: Consider verifying the actual http→https conversion.The test asserts that initialization completes successfully but doesn't verify that the domain was actually converted from
http://tohttps://. While this may be a smoke test, adding an assertion to check the final domain value would strengthen the test's value.Example enhancement:
test('should convert http:// to https:// in auth domain', () async { final sdk = await initializeKindeFlutterSdkForTest( authDomain: 'http://test.kinde.com', authClientId: 'test_client_id', loginRedirectUri: 'myapp://callback', logoutRedirectUri: 'myapp://logout', dio: dio, ); expect(sdk, isNotNull); // Add assertion to verify the domain was normalized // expect(sdk.authDomain, equals('test.kinde.com')); // or similar });test/token_utils_test.dart (2)
154-165: Duplicate test case can be removed.This test (
should handle user details with all fields populated) is functionally identical toshould return user details from id token(lines 125-136) - both useAuthStateFixtures.createValid()and verify the same fields with identical assertions.Consider removing this duplicate test:
- test('should handle user details with all fields populated', () { - tokenUtils.authState = AuthStateFixtures.createValid(); - - final userDetails = tokenUtils.getUserDetails(); - - expect(userDetails, isNotNull); - expect(userDetails!.id, JwtTestHelper.testUserId); - expect(userDetails.email, JwtTestHelper.testEmail); - expect(userDetails.givenName, JwtTestHelper.testGivenName); - expect(userDetails.familyName, JwtTestHelper.testFamilyName); - expect(userDetails.picture, JwtTestHelper.testPicture); - });
253-259: Duplicate test case provides minimal additional value.This test (
should return org code from token) is essentially the same asshould return organization codeabove (lines 245-251), just with a different org code value ('my_org'vs'org_acme').Consider consolidating into a single parameterized test or removing:
- test('should return org code from token', () { - tokenUtils.authState = AuthStateFixtures.createWithOrganization('my_org'); - - final org = tokenUtils.getOrganization(); - - expect(org.orgCode, 'my_org'); - });test/kinde_web_test.dart (2)
118-126: Tautological test provides no value.This test only verifies that constant values equal themselves (
minLength >= 43,maxLength <= 128). It doesn't test any SDK code behavior.Consider removing this test or replacing it with an actual test that validates the code verifier length from the SDK's implementation:
- test('code verifier length requirements per RFC 7636', () { - // RFC 7636 specifies code_verifier length between 43-128 characters - const minLength = 43; - const maxLength = 128; - - expect(minLength, lessThanOrEqualTo(maxLength)); - expect(minLength, greaterThanOrEqualTo(43)); - expect(maxLength, lessThanOrEqualTo(128)); - });
129-140: Tautological test provides no value.This test only verifies that a constant equals itself (
expectedMinLength >= 16). It doesn't test any actual SDK functionality.Consider removing this test group or replacing it with actual tests that verify the SDK's auth state generation behavior:
- group('Auth State Generation', () { - test('auth state should be sufficiently random', () { - // Auth state is used to prevent CSRF attacks - // It should be unpredictable and unique per request - - // Verify expected length based on typical implementations - // Most implementations use 16-32 bytes encoded as hex or base64 - const expectedMinLength = 16; // 16 bytes minimum for security - - expect(expectedMinLength, greaterThanOrEqualTo(16)); - }); - });test/store_test.dart (1)
12-15: Consider adding Store.init() to setUp for consistency.Currently
Store.init()is called at the beginning of each test. Moving it to asetUpblock would reduce code duplication and ensure consistent initialization.setUpAll(() { // Initialize mock platform channels for secure storage mockChannels.setupMockChannel(); }); + setUp(() async { + await Store.init(); + }); + group('Store', () { group('Singleton Pattern', () { test('should return same instance', () { final instance1 = Store.instance;Then remove individual
await Store.init();calls from each test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
test/additional_params_test.dart(1 hunks)test/auth_config_test.dart(1 hunks)test/auth_state_test.dart(1 hunks)test/helpers_test.dart(1 hunks)test/kinde_error_test.dart(1 hunks)test/kinde_flutter_sdk_core_test.dart(1 hunks)test/kinde_web_test.dart(1 hunks)test/mock_channels.dart(1 hunks)test/refresh_token_interceptor_test.dart(1 hunks)test/store_test.dart(1 hunks)test/test_helpers/auth_state_fixtures.dart(1 hunks)test/test_helpers/jwt_test_helper.dart(1 hunks)test/test_helpers/mock_store.dart(1 hunks)test/token_api_test.dart(1 hunks)test/token_utils_test.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:110-124
Timestamp: 2025-06-20T10:43:27.820Z
Learning: The PR #27 for the Flutter starter kit example app is designed to work with Kinde Flutter SDK version 1.0.4, where the register() method will return a token if registration is successful. The current build errors are expected since the code targets a future SDK version.
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:79-94
Timestamp: 2025-06-20T10:44:49.809Z
Learning: In the Kinde Flutter SDK, version 1.0.4 will change the login() method to return a token when login is successful, rather than returning void. Code in this repository may be written to anticipate this future API change.
📚 Learning: 2025-11-27T10:12:42.833Z
Learnt from: KeeganBeuthin
Repo: kinde-oss/kinde-flutter-sdk PR: 52
File: lib/src/store/store.dart:54-57
Timestamp: 2025-11-27T10:12:42.833Z
Learning: In the Kinde Flutter SDK's Store class (lib/src/store/store.dart), the authState and keys setters intentionally use a fire-and-forget pattern when writing to flutter_secure_storage. This maintains API compatibility with the previous Hive-based implementation and avoids introducing breaking changes by keeping the setters synchronous rather than converting them to async methods.
Applied to files:
test/auth_state_test.darttest/mock_channels.darttest/store_test.darttest/test_helpers/auth_state_fixtures.darttest/test_helpers/mock_store.dart
📚 Learning: 2025-11-27T10:13:04.736Z
Learnt from: KeeganBeuthin
Repo: kinde-oss/kinde-flutter-sdk PR: 52
File: lib/src/store/store.dart:44-47
Timestamp: 2025-11-27T10:13:04.736Z
Learning: In the kinde-flutter-sdk Store class (lib/src/store/store.dart), the authState and keys setters use a fire-and-forget pattern for writing to flutter_secure_storage to maintain backward compatibility with the previous Hive-based implementation, which also had synchronous setters. This is an intentional design decision to avoid breaking changes.
Applied to files:
test/auth_state_test.darttest/mock_channels.darttest/store_test.darttest/kinde_flutter_sdk_core_test.darttest/test_helpers/auth_state_fixtures.darttest/test_helpers/mock_store.dart
📚 Learning: 2025-06-20T10:43:27.820Z
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:110-124
Timestamp: 2025-06-20T10:43:27.820Z
Learning: The PR #27 for the Flutter starter kit example app is designed to work with Kinde Flutter SDK version 1.0.4, where the register() method will return a token if registration is successful. The current build errors are expected since the code targets a future SDK version.
Applied to files:
test/auth_state_test.darttest/kinde_web_test.darttest/kinde_flutter_sdk_core_test.darttest/kinde_error_test.darttest/token_api_test.dart
📚 Learning: 2025-06-20T10:44:49.809Z
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:79-94
Timestamp: 2025-06-20T10:44:49.809Z
Learning: In the Kinde Flutter SDK, version 1.0.4 will change the login() method to return a token when login is successful, rather than returning void. Code in this repository may be written to anticipate this future API change.
Applied to files:
test/kinde_flutter_sdk_core_test.darttest/token_api_test.dart
🔇 Additional comments (18)
test/token_utils_test.dart (2)
23-28: LGTM!Good use of
setUpto initialize a freshTestTokenUtilsinstance for each test, ensuring test isolation.
302-376: LGTM!Excellent coverage of feature flag scenarios including type handling, default values, missing flags, and type mismatch errors. The tests properly verify both the flag metadata (
code,type,isDefault) and values.test/kinde_web_test.dart (2)
238-284: LGTM!Solid test coverage for URL state extraction including URL-encoded parameters, missing required parameters, and error response parsing. These tests validate actual URL parsing behavior that the SDK would rely on.
8-59: LGTM!Good coverage of KindeError codes for web flows. These tests validate that error code constants exist and match expected string values.
test/test_helpers/mock_store.dart (1)
6-24: LGTM!Clean and minimal mock implementation that avoids Flutter secure storage dependencies for unit testing. The synchronous setters align with the real Store's API (which uses fire-and-forget pattern per learnings).
test/kinde_error_test.dart (3)
164-266: LGTM!Excellent coverage of DioException handling including all timeout types, connection errors, cancellation, and nested KindeError extraction. The test at line 253-265 for nested errors is particularly valuable for ensuring proper error unwrapping.
268-309: LGTM!Good coverage of FormatException handling including OAuth state mismatch detection, JSON error extraction from messages, and graceful fallback for non-JSON or malformed content.
46-74: LGTM!Well-structured equality tests covering both positive and negative cases, plus hashCode consistency verification.
test/store_test.dart (3)
350-364: Verify empty token string handling is intentional.This test validates that empty strings can be stored for
accessToken,idToken,refreshToken, andscope. In practice, empty tokens would likely indicate an invalid state.Consider whether this edge case reflects intended behavior or if the SDK should reject/handle empty tokens differently. If empty strings are valid (e.g., representing "no token"), this test is appropriate.
308-325: LGTM!Good test for verifying the fire-and-forget pattern handles rapid consecutive updates correctly, with the final state reflecting the last update. Based on learnings, this aligns with the intentional design decision for backward compatibility.
253-306: LGTM!Good JSON serialization round-trip tests for both AuthState and Keys. The comment on line 295 appropriately notes the nested serialization nuance for Keys.
test/test_helpers/jwt_test_helper.dart (6)
9-17: LGTM! Well-defined test constants.The test constants provide clear, reusable values for common JWT claims and are appropriately defined as const.
20-52: LGTM! Flexible token creation with sensible defaults.The method provides good flexibility through optional parameters while maintaining sensible defaults for all required claims.
108-143: LGTM! Convenient helper methods for common test scenarios.These specialized factory methods make it easy to create tokens for specific test cases without manually specifying all parameters.
145-150: LGTM! Comprehensive feature flag examples.The test feature flags cover multiple data types (boolean, string, integer) and follow the expected structure with type and value keys.
152-167: LGTM! Correct JWT construction for testing.The JWT structure and base64URL encoding are correct. Using a placeholder signature is appropriate since signature validation is not required for these test scenarios.
170-174: LGTM! Nice convenience extension.The extension method provides a fluent, readable way to create expiring tokens in tests (e.g.,
Duration(hours: 1).toExpiringToken()).test/test_helpers/auth_state_fixtures.dart (1)
8-131: LGTM! Comprehensive and well-structured test fixtures.The AuthStateFixtures class provides excellent coverage of common testing scenarios:
- Various expiration states (valid, expired, expiring in duration)
- Different token combinations (with/without ID token, empty state)
- Domain-specific scenarios (feature flags, permissions, organizations)
- JSON deserialization testing (expires_in vs. ISO date formats)
Each factory method is clear, concise, and appropriately delegates to JwtTestHelper for token generation. The use of const constructors (line 62) and sensible defaults throughout makes these fixtures easy to use in tests.
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
Requested some changes for clean up and to streamline the tests.
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
Requesting some changes with how we test time sensitive logic.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
test/token_utils_test.dart (1)
154-165: Remove or differentiate the duplicategetUserDetailshappy‑path test
test('should handle user details with all fields populated', ...)is effectively identical totest('should return user details from id token', ...)above (same fixture, same expectations). Consider removing the duplicate or varying the scenario (e.g., partially populated fields) to add extra coverage instead of repetition.test/auth_state_test.dart (2)
36-110: fromJson tests give good coverage of JSON shapes and edge casesNice coverage of:
expires_invs ISO timestamp inputs- missing
grant_typedefaulting- missing optional fields
- zero and negative
expires_inbehaviorThis should catch most regressions in
AuthState.fromJson. The existing feedback about moving away fromDateTime.now-based tolerances via a test clock still applies; beyond that, nothing additional from my side.
161-201: Consider makingisExpiredtests fully deterministic with a test clockThe
isExpiredgroup nicely covers the main branches (no token, no expiry, clearly expired, clearly not expired, near‑boundary). A couple of the “near now” cases (e.g., expiring in 1 second) still depend on real wall‑clock timing and could, in extreme slow CI environments, become flaky.Once
AuthState.isExpiredis wired to a testable clock (as discussed in prior comments), it would be worth wrapping these in a fixed clock (withClock(...)) or letting the fixtures accept an explicit base time so the tests are entirely deterministic.
🧹 Nitpick comments (3)
test/token_utils_test.dart (2)
11-21: Consider removing or usingsetAuthStateto avoid dead code
setAuthStateis never used in this file; tests settokenUtils.authStatedirectly. Either switch tests to use the helper or drop it to keep the test helper minimal.
55-69: Tighten and/or de-coupleparseTokenerror expectations from implementation detailsThe malformed-token test asserts a raw
RangeError, while the invalid‑base64 test acceptsthrowsA(anything). That combination is a bit inconsistent and couples tests to the current internal implementation. You might want to either:
- assert a more domain‑level error type (e.g., a
KindeError) where appropriate, or- relax the first case to “any exception” if you only care that malformed tokens fail.
This is optional but will make future refactors of
parseTokenless brittle.test/auth_state_test.dart (1)
112-159: Tighten the round‑trip toJson/fromJson test and clean up the commentThe round‑trip test is good, but you can make it clearer and a bit stronger:
- The comment about converting
expires_inno longer matches what the code does.- You’re not currently asserting that
accessTokenExpirationDateTimeis preserved through the round‑trip, even thoughfromJsoncan reconstruct it from the ISO string.Consider this small refactor:
test('should round-trip through JSON correctly', () { final original = AuthStateFixtures.createValid(); final json = original.toJson(); - - // Convert expires_in format for fromJson compatibility - final jsonForParsing = Map<String, dynamic>.from(json); - - final restored = AuthState.fromJson(jsonForParsing); + final restored = AuthState.fromJson( + Map<String, dynamic>.from(json), + ); expect(restored.accessToken, original.accessToken); expect(restored.refreshToken, original.refreshToken); expect(restored.idToken, original.idToken); expect(restored.grantType, original.grantType); expect(restored.scope, original.scope); + expect( + restored.accessTokenExpirationDateTime, + original.accessTokenExpirationDateTime, + ); });This keeps the intent the same while verifying the expiry field and removing an outdated comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/auth_state_test.dart(1 hunks)test/kinde_flutter_sdk_core_test.dart(1 hunks)test/token_utils_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/kinde_flutter_sdk_core_test.dart
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:110-124
Timestamp: 2025-06-20T10:43:27.820Z
Learning: The PR #27 for the Flutter starter kit example app is designed to work with Kinde Flutter SDK version 1.0.4, where the register() method will return a token if registration is successful. The current build errors are expected since the code targets a future SDK version.
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:79-94
Timestamp: 2025-06-20T10:44:49.809Z
Learning: In the Kinde Flutter SDK, version 1.0.4 will change the login() method to return a token when login is successful, rather than returning void. Code in this repository may be written to anticipate this future API change.
📚 Learning: 2025-11-27T10:12:42.833Z
Learnt from: KeeganBeuthin
Repo: kinde-oss/kinde-flutter-sdk PR: 52
File: lib/src/store/store.dart:54-57
Timestamp: 2025-11-27T10:12:42.833Z
Learning: In the Kinde Flutter SDK's Store class (lib/src/store/store.dart), the authState and keys setters intentionally use a fire-and-forget pattern when writing to flutter_secure_storage. This maintains API compatibility with the previous Hive-based implementation and avoids introducing breaking changes by keeping the setters synchronous rather than converting them to async methods.
Applied to files:
test/auth_state_test.dart
📚 Learning: 2025-11-27T10:13:04.736Z
Learnt from: KeeganBeuthin
Repo: kinde-oss/kinde-flutter-sdk PR: 52
File: lib/src/store/store.dart:44-47
Timestamp: 2025-11-27T10:13:04.736Z
Learning: In the kinde-flutter-sdk Store class (lib/src/store/store.dart), the authState and keys setters use a fire-and-forget pattern for writing to flutter_secure_storage to maintain backward compatibility with the previous Hive-based implementation, which also had synchronous setters. This is an intentional design decision to avoid breaking changes.
Applied to files:
test/auth_state_test.dart
📚 Learning: 2025-06-20T10:43:27.820Z
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:110-124
Timestamp: 2025-06-20T10:43:27.820Z
Learning: The PR #27 for the Flutter starter kit example app is designed to work with Kinde Flutter SDK version 1.0.4, where the register() method will return a token if registration is successful. The current build errors are expected since the code targets a future SDK version.
Applied to files:
test/auth_state_test.dart
📚 Learning: 2025-06-20T10:44:49.809Z
Learnt from: kartuli98
Repo: kinde-oss/kinde-flutter-sdk PR: 27
File: example/lib/state/app_state_manager.dart:79-94
Timestamp: 2025-06-20T10:44:49.809Z
Learning: In the Kinde Flutter SDK, version 1.0.4 will change the login() method to return a token when login is successful, rather than returning void. Code in this repository may be written to anticipate this future API change.
Applied to files:
test/auth_state_test.dart
🔇 Additional comments (3)
test/auth_state_test.dart (3)
10-34: Constructor tests correctly cover full and default initializationThese tests validate both a fully-populated
AuthStateand the default constructor (includinggrantTypedefault), which is exactly what you want here. No issues spotted.
203-229: getToken tests cover both token types and null cases wellThese tests exercise both
TokenType.accessTokenandTokenType.idToken, plus the null branches, which is exactly the behavior surface ofgetToken. Looks solid.
231-306: createRequestTokenParam tests thoroughly cover inclusion/exclusion rulesGood coverage of:
- happy‑path map creation
- excluding null
refreshToken/grantType- excluding empty or null
scope- fully empty case
This should give high confidence that request params aren’t polluted with null/empty values. No further changes needed from my side.
|
Hello @victoreronmosele thank you for the feedback. I have taken this into account and removed the accidental duplicate tests as well as added the requested assertions as well as improving the logout tests. With regard to using withClock, your suggestions are certainly valid, however that would require modifying auth_state.dart which is outside of the scope of this particular pr. I do agree that it would make tests more deterministic and we can look at making that change in a follow up pr. With regard to the positive getToken test, I attempted this, but Please let me know if you have any further feedback bro :) |
You're right. We can improve this in a follow up PR. |
|
Reviewing the updates. |
We can also improve this in a different PR. I'll also look into it. |
|
Approved! |
KeeganBeuthin
changed the title
Created comprehensive, maintainable test suite for UsersApi following pragmatic, enterprise-grade principles with zero over-engineering. ## Key Achievements: - Upgraded 5 API methods from placeholder to comprehensive tests - Increased test count from 5 → 50 tests (+900%) - Removed all TODO comments - Implemented proper HTTP mocking with http_mock_adapter - Added comprehensive error scenarios (400, 401, 403, 404, 500) - Added request validation tests - Used inline fixtures (no premature abstraction) - Followed AAA pattern throughout - Zero linter errors ## Test Coverage: - createUser: 13 tests (success, errors, request validation) - getUserData: 7 tests (success, errors, request validation) - getUsers: 14 tests (success, pagination, filtering, sorting, errors) - updateUser: 7 tests (success, errors, request validation) - deleteUser: 9 tests (success, errors, request validation) ## Quality Standards Met: ✅ Proper HTTP mocking (not generic DioAdapterMock) ✅ Inline, readable test data (no massive fixtures file) ✅ AAA pattern (Arrange, Act, Assert) ✅ Multiple specific assertions (not just isNotNull) ✅ Clear, descriptive test names ✅ Request structure validation ✅ Response structure validation ✅ Edge case coverage (pagination, optional params) ✅ Aligned with PR kinde-oss#53 patterns ✅ Self-reviewed and reflected before implementation ## Documentation: - TEST_MODERNIZATION_APPROACH.md: Pragmatic strategy & rationale - PHASE_1_PROTOTYPE_COMPLETE.md: Comprehensive completion report ## Next Steps: - User review & test execution - Evaluate patterns & repetition - Decide on helper extraction (if needed) - Apply template to remaining 13 API test files This prototype serves as the template for modernizing all 14 API test files with enterprise-grade quality and maintainability.
dtoxvanilla1991
left a comment
dtoxvanilla1991
left a comment
There was a problem hiding this comment.
Good stuff. Well done. Left several 👣
|
|
||
| /// Mock store for testing that doesn't require Flutter secure storage. | ||
| /// This provides direct access to auth state manipulation for unit tests. | ||
| class MockStore { |
There was a problem hiding this comment.
Dead code 💀 - file never imported.
MockStore is not referenced by any test file in this PR or the existing codebase. A grep across the entire workspace (including ignored files) returns zero usages of MockStore.
This was flagged by @victoreronmosele in a prior review thread and the thread was resolved, but, interestingly, the file still ships in this final commit. Please remove it entirely to avoid dead code accumulation.
| test('code verifier length requirements per RFC 7636', () { | ||
| // RFC 7636 specifies code_verifier length between 43-128 characters | ||
| const minLength = 43; | ||
| const maxLength = 128; | ||
|
|
||
| expect(minLength, lessThanOrEqualTo(maxLength)); | ||
| expect(minLength, greaterThanOrEqualTo(43)); | ||
| expect(maxLength, lessThanOrEqualTo(128)); | ||
| }); |
There was a problem hiding this comment.
Tautological test — asserts constants against themselves, tests no SDK behavior.
This test declares minLength = 43 then asserts expect(minLength, greaterThanOrEqualTo(43)). This will always pass regardless of the SDK's actual implementation. It doesn't invoke any code verifier generation logic.
Either remove this test or replace it with an assertion that calls into actual SDK code to verify the generated code verifier's length falls within the RFC 7636 range (43–128 chars). If web crypto isn't available in VM tests, document that as a known gap rather than shipping a no-op assertion.
| group('Auth State Generation', () { | ||
| test('auth state should be sufficiently random', () { | ||
| // Auth state is used to prevent CSRF attacks | ||
| // It should be unpredictable and unique per request | ||
|
|
||
| // Verify expected length based on typical implementations | ||
| // Most implementations use 16-32 bytes encoded as hex or base64 | ||
| const expectedMinLength = 16; // 16 bytes minimum for security | ||
|
|
||
| expect(expectedMinLength, greaterThanOrEqualTo(16)); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Same issue — tautological test.
expect(expectedMinLength, greaterThanOrEqualTo(16)) where expectedMinLength is a const 16 defined two lines above. This tests nothing about the SDK's auth state generation. Remove or replace with a real assertion against actual generated state values.
| void setAuthState(AuthState? state) { | ||
| authState = state; | ||
| } |
There was a problem hiding this comment.
Dead code ☠️ - setAuthState is never called.
All tests in this file set tokenUtils.authState directly via the public field. This helper method adds no value. Remove it to keep the test helper minimal - the public field is sufficient.
|
|
||
| test('should throw session-expired-or-invalid when auth state invalid', () async { | ||
| // Setting authState without proper validation still triggers error | ||
| sdk.authState = AuthState( |
There was a problem hiding this comment.
Advisory: sdk.authState = writes to the TokenUtils mixin field, not to Store.
In prod, KindeFlutterSDK.authState getter reads from _store.authState, but this assignment writes to the mixin's @protected AuthState? authState field. This means the test is exercising a disconnected state path - the Store's cached state remains null while the mixin's field gets populated.
This doesn't break anything in this PR (since the subsequent getToken() call fails for other reasons), but it's worth noting for future test authors: positive-path tests that rely on sdk.authState = ... won't reflect the real runtime behavior where state flows through Store. Consider adding a comment here noting this limitation, or in a follow-up PR, add a setter override in KindeFlutterSDK that delegates to Store.
3 participants
Changes
New Test Infrastructure
JwtTestHelper- Factory for creating mock JWT tokens with customizable claimsAuthStateFixtures- Pre-configured AuthState instances for common test scenariosEnhanced
MockChannels- Full secure storage simulation (read/write/delete/deleteAll)New Test Files
auth_state_test.dart- Constructor, JSON serialization, expiration, token retrievaltoken_utils_test.dart- Claims, permissions, feature flags, organizationskinde_error_test.dart- Error factory, Dio/Platform/Format exceptionsauth_config_test.dart- URL validation, scope handlingadditional_params_test.dart- Web/auth request parameter conversionhelpers_test.dart- URL safety validationrefresh_token_interceptor_test.dart- Token refresh on 403, error handlingtoken_api_test.dart- OAuth token endpoint interactionsstore_test.dart- Secure storage persistencekinde_web_test.dart- Web OAuth flows, PKCE, error codes