-
Notifications
You must be signed in to change notification settings - Fork 6
test: add tests for TransactionDetailsPage
#316
test: add tests for TransactionDetailsPage
#316
Conversation
| import '../../helpers/mocks.dart'; | ||
| import '../../helpers/test_data.dart'; | ||
| import '../../helpers/widget_helpers.dart'; | ||
| import 'transaction_tile_test.dart'; |
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.
Instead of importing another test file, you could put the shared data in the test_data.dart file and import that in this test.
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.
+1!
| sealed class MockTransactionNotifierType { | ||
| const MockTransactionNotifierType(); | ||
| } | ||
|
|
||
| class MockTransactionNotifierWithData extends MockTransactionNotifierType { | ||
| const MockTransactionNotifierWithData({required this.transactionType}); | ||
|
|
||
| final TransactionType transactionType; | ||
| } | ||
|
|
||
| class MockTransactionNotifierWithNullData extends MockTransactionNotifierType { | ||
| const MockTransactionNotifierWithNullData(); | ||
| } | ||
|
|
||
| class MockTransactionNotifierWithError extends MockTransactionNotifierType { | ||
| const MockTransactionNotifierWithError(); | ||
| } |
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.
this code is duplicated from the TransactionTile tests, let's move this to mocks.dart!
| sealed class MockTransactionNotifierType { | ||
| const MockTransactionNotifierType(); | ||
| } | ||
|
|
||
| class MockTransactionNotifierWithData extends MockTransactionNotifierType { | ||
| const MockTransactionNotifierWithData({required this.transactionType}); | ||
|
|
||
| final TransactionType transactionType; | ||
| } | ||
|
|
||
| class MockTransactionNotifierWithNullData extends MockTransactionNotifierType { | ||
| const MockTransactionNotifierWithNullData(); | ||
| } | ||
|
|
||
| class MockTransactionNotifierWithError extends MockTransactionNotifierType { | ||
| const MockTransactionNotifierWithError(); | ||
| } |
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.
These lines are still duplicated in transaction_tile_test.dart.
didpay/test/features/transaction/transaction_tile_test.dart
Lines 186 to 202 in 0bc1feb
| sealed class MockTransactionNotifierType { | |
| const MockTransactionNotifierType(); | |
| } | |
| class MockTransactionNotifierWithData extends MockTransactionNotifierType { | |
| const MockTransactionNotifierWithData({required this.transactionType}); | |
| final TransactionType transactionType; | |
| } | |
| class MockTransactionNotifierWithNullData extends MockTransactionNotifierType { | |
| const MockTransactionNotifierWithNullData(); | |
| } | |
| class MockTransactionNotifierWithError extends MockTransactionNotifierType { | |
| const MockTransactionNotifierWithError(); | |
| } |
They can be removed from the transaction_tile_test.dart so that it only uses the ones from mocks.dart.
Removed duplicate code.
victoreronmosele
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏽
| expect( | ||
| find.text('100.01'), | ||
| findsOneWidget, | ||
| ); // Adjust to your currency setup. |
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.
| expect(find.text('Bad state: Error loading transaction'), findsOneWidget); | ||
| }); | ||
| }); | ||
| } No newline at end of file |
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.
nit: add missing newline here!
ethanwlee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some small nits but overall looks good! could you change the pr title to match the structure of other prs? thanks!
TransactionDetailsPage
done. |
|
@mohitrajsinha can you address the nits so I can merge |
Hey @ethan-tbd check out this pull request. Do let me know if there is anything to fix.
Transaction detail page test added.
closes #289