Conversation
WalkthroughAdds a new Dart test validating LaunchesRepositoryImpl behavior by mocking LaunchesDataSource. Tests cover success, error, and loading cases for getLaunches and getLaunch, verifying mapping from network models to resource models and that non-success results throw exceptions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ 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. Comment |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
+ Coverage 80.83% 81.61% +0.77%
==========================================
Files 96 96
Lines 1931 1931
==========================================
+ Hits 1561 1576 +15
+ Misses 370 355 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
test/repository/launches_repository_impl_test.dart (3)
24-42: Success case is good; add a couple stronger assertions and verify the call.Assert length/flightNumber and verify the data source is called once.
Apply:
final result = await repository.getLaunches(); - expect(result, isA<List<LaunchResource>>()); + expect(result, isA<List<LaunchResource>>()); + expect(result, hasLength(1)); expect(result.first.missionName, equals('Test')); + expect(result.first.flightNumber, equals(1)); + verify(() => mockDataSource.getLaunches( + hasId: any(named: 'hasId'), + limit: any(named: 'limit'), + offset: any(named: 'offset'), + launchYear: any(named: 'launchYear'), + launchSuccess: any(named: 'launchSuccess'), + order: any(named: 'order'), + )).called(1);
82-93: Success path looks solid; add one more assert and verify interaction.final result = await repository.getLaunch(42); expect(result, isA<LaunchFullResource>()); expect(result.missionName, equals('Falcon')); + expect(result.flightNumber, equals(42)); + verify(() => mockDataSource.getLaunch(42)).called(1);
27-34: Be explicit with matcher types for named args (prevents mocktail type mismatches).If any of these params are non-dynamic (e.g., int?, bool?, String?, or an enum), prefer
any<int?>(),any<bool?>(), etc., or pass concrete values. Iforderis an enum, you may need a real value or a registered fallback.Run to confirm exact parameter types:
#!/bin/bash # Locate the LaunchesDataSource.getLaunches signature rg -nP 'abstract\s+class\s+LaunchesDataSource|class\s+LaunchesNetworkDataSource' -C3 rg -nP 'getLaunches\s*\(' -C2Also applies to: 45-52, 63-70
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/repository/launches_repository_impl_test.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: App
- GitHub Check: Flutter Integration Tests (31)
🔇 Additional comments (2)
test/repository/launches_repository_impl_test.dart (2)
7-8: Confirm LaunchesRepositoryImpl import/export path.
LaunchesRepositoryImplis referenced but onlyrepository/launches_repository.dartis imported. Ensure the impl is exported from there or import the concrete impl file directly to avoid unresolved symbol errors.Run to locate the implementation and its export:
#!/bin/bash # Find the impl and where it's exported from rg -nP 'class\s+LaunchesRepositoryImpl\b|export .+launches_repository_impl' -C2
53-54: Verify ApiResult constructors are const-safe.If
ApiResult.error/ApiResult.loadingaren’t const constructors, dropconstto avoid analysis errors; keep style consistent across tests.#!/bin/bash # Check ApiResult constructors rg -nP 'class\s+ApiResult|factory\s+ApiResult\.' -C2Also applies to: 71-72, 97-98, 108-109
| test('throws Exception when error', () async { | ||
| when(() => mockDataSource.getLaunches( | ||
| hasId: any(named: 'hasId'), | ||
| limit: any(named: 'limit'), | ||
| offset: any(named: 'offset'), | ||
| launchYear: any(named: 'launchYear'), | ||
| launchSuccess: any(named: 'launchSuccess'), | ||
| order: any(named: 'order'), | ||
| )).thenAnswer( | ||
| (_) async => const ApiResult.error('Something went wrong'), | ||
| ); | ||
|
|
||
| expect( | ||
| () => repository.getLaunches(), | ||
| throwsA(isA<Exception>()), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Fix async exception expectation (use expectLater with the Future).
expect(() => future, throwsA(...)) checks sync throws. For Futures, pass the Future to expectLater (and await it).
- expect(
- () => repository.getLaunches(),
- throwsA(isA<Exception>()),
- );
+ await expectLater(
+ repository.getLaunches(),
+ throwsA(isA<Exception>()),
+ );📝 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.
| test('throws Exception when error', () async { | |
| when(() => mockDataSource.getLaunches( | |
| hasId: any(named: 'hasId'), | |
| limit: any(named: 'limit'), | |
| offset: any(named: 'offset'), | |
| launchYear: any(named: 'launchYear'), | |
| launchSuccess: any(named: 'launchSuccess'), | |
| order: any(named: 'order'), | |
| )).thenAnswer( | |
| (_) async => const ApiResult.error('Something went wrong'), | |
| ); | |
| expect( | |
| () => repository.getLaunches(), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); | |
| test('throws Exception when error', () async { | |
| when(() => mockDataSource.getLaunches( | |
| hasId: any(named: 'hasId'), | |
| limit: any(named: 'limit'), | |
| offset: any(named: 'offset'), | |
| launchYear: any(named: 'launchYear'), | |
| launchSuccess: any(named: 'launchSuccess'), | |
| order: any(named: 'order'), | |
| )).thenAnswer( | |
| (_) async => const ApiResult.error('Something went wrong'), | |
| ); | |
| await expectLater( | |
| repository.getLaunches(), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In test/repository/launches_repository_impl_test.dart around lines 44 to 60, the
test uses expect(() => repository.getLaunches(), throwsA(...)) which treats the
call as synchronous; change it to await expectLater(repository.getLaunches(),
throwsA(isA<Exception>())); i.e., pass the Future returned by
repository.getLaunches() to expectLater and await the expect so the async
exception is correctly asserted.
| test('throws Exception when loading', () async { | ||
| when(() => mockDataSource.getLaunches( | ||
| hasId: any(named: 'hasId'), | ||
| limit: any(named: 'limit'), | ||
| offset: any(named: 'offset'), | ||
| launchYear: any(named: 'launchYear'), | ||
| launchSuccess: any(named: 'launchSuccess'), | ||
| order: any(named: 'order'), | ||
| )).thenAnswer( | ||
| (_) async => const ApiResult.loading(), | ||
| ); | ||
|
|
||
| expect( | ||
| () => repository.getLaunches(), | ||
| throwsA(isA<Exception>()), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Same async throws issue here.
- expect(
- () => repository.getLaunches(),
- throwsA(isA<Exception>()),
- );
+ await expectLater(
+ repository.getLaunches(),
+ throwsA(isA<Exception>()),
+ );📝 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.
| test('throws Exception when loading', () async { | |
| when(() => mockDataSource.getLaunches( | |
| hasId: any(named: 'hasId'), | |
| limit: any(named: 'limit'), | |
| offset: any(named: 'offset'), | |
| launchYear: any(named: 'launchYear'), | |
| launchSuccess: any(named: 'launchSuccess'), | |
| order: any(named: 'order'), | |
| )).thenAnswer( | |
| (_) async => const ApiResult.loading(), | |
| ); | |
| expect( | |
| () => repository.getLaunches(), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); | |
| test('throws Exception when loading', () async { | |
| when(() => mockDataSource.getLaunches( | |
| hasId: any(named: 'hasId'), | |
| limit: any(named: 'limit'), | |
| offset: any(named: 'offset'), | |
| launchYear: any(named: 'launchYear'), | |
| launchSuccess: any(named: 'launchSuccess'), | |
| order: any(named: 'order'), | |
| )).thenAnswer( | |
| (_) async => const ApiResult.loading(), | |
| ); | |
| await expectLater( | |
| repository.getLaunches(), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In test/repository/launches_repository_impl_test.dart around lines 62 to 78, the
test passes a synchronous callback to expect for an async method which causes an
incorrect assertion; change the expectation to handle a Future by either passing
the Future directly to expect (e.g., expect(repository.getLaunches(),
throwsA(...))) or by using an async closure (e.g., expect(() async => await
repository.getLaunches(), throwsA(...))) so the async error is awaited and
properly asserted.
| test('throws Exception when error', () async { | ||
| when(() => mockDataSource.getLaunch(42)).thenAnswer( | ||
| (_) async => const ApiResult.error('Not found'), | ||
| ); | ||
|
|
||
| expect( | ||
| () => repository.getLaunch(42), | ||
| throwsA(isA<Exception>()), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Fix async exception expectation in getLaunch(error).
- expect(
- () => repository.getLaunch(42),
- throwsA(isA<Exception>()),
- );
+ await expectLater(
+ repository.getLaunch(42),
+ throwsA(isA<Exception>()),
+ );📝 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.
| test('throws Exception when error', () async { | |
| when(() => mockDataSource.getLaunch(42)).thenAnswer( | |
| (_) async => const ApiResult.error('Not found'), | |
| ); | |
| expect( | |
| () => repository.getLaunch(42), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); | |
| test('throws Exception when error', () async { | |
| when(() => mockDataSource.getLaunch(42)).thenAnswer( | |
| (_) async => const ApiResult.error('Not found'), | |
| ); | |
| await expectLater( | |
| repository.getLaunch(42), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In test/repository/launches_repository_impl_test.dart around lines 95 to 104,
the test uses a synchronous closure in expect for an async call; change the
expectation to handle the Future by making the closure async and awaiting the
call (e.g. expect(() async => await repository.getLaunch(42),
throwsA(isA<Exception>()))) or alternatively use
expectLater(repository.getLaunch(42), throwsA(isA<Exception>())) so the async
exception is correctly asserted.
| test('throws Exception when loading', () async { | ||
| when(() => mockDataSource.getLaunch(42)).thenAnswer( | ||
| (_) async => const ApiResult.loading(), | ||
| ); | ||
|
|
||
| expect( | ||
| () => repository.getLaunch(42), | ||
| throwsA(isA<Exception>()), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Fix async exception expectation in getLaunch(loading).
- expect(
- () => repository.getLaunch(42),
- throwsA(isA<Exception>()),
- );
+ await expectLater(
+ repository.getLaunch(42),
+ throwsA(isA<Exception>()),
+ );📝 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.
| test('throws Exception when loading', () async { | |
| when(() => mockDataSource.getLaunch(42)).thenAnswer( | |
| (_) async => const ApiResult.loading(), | |
| ); | |
| expect( | |
| () => repository.getLaunch(42), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); | |
| test('throws Exception when loading', () async { | |
| when(() => mockDataSource.getLaunch(42)).thenAnswer( | |
| (_) async => const ApiResult.loading(), | |
| ); | |
| await expectLater( | |
| repository.getLaunch(42), | |
| throwsA(isA<Exception>()), | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In test/repository/launches_repository_impl_test.dart around lines 106 to 115,
the test is asserting that repository.getLaunch(42) throws, but it passes a
non-awaited call to expect; change the assertion to an async-aware form so the
thrown exception from the Future is observed. Replace expect(() =>
repository.getLaunch(42), throwsA(...)) with either expect(() async => await
repository.getLaunch(42), throwsA(isA<Exception>())); or
expectLater(repository.getLaunch(42), throwsA(isA<Exception>())); ensuring the
repository call is awaited or passed as a Future so the test correctly catches
the exception.
1 participant
Summary by CodeRabbit