-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add UI Tests for CurrencyDropdown Widget #312
Add UI Tests for CurrencyDropdown Widget #312
Conversation
@@ -18,6 +18,7 @@ class CurrencyDropdown extends HookConsumerWidget { | |||
Widget build(BuildContext context, WidgetRef ref) => Directionality( | |||
textDirection: TextDirection.rtl, | |||
child: ElevatedButton.icon( | |||
key: const Key('currencyDropdownButton'), |
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.
please remove this line as keys aren't being used in tests
const testCurrency = 'USD'; | ||
|
||
setUp(() { | ||
// Initialize mockState with a non-null offeringsMap |
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.
comments in test file are not required!
Widget createWidgetUnderTest() { | ||
return WidgetHelpers.testableWidget( | ||
// Use WidgetHelpers for consistent test setup | ||
child: CurrencyDropdown( | ||
paymentCurrency: testCurrency, | ||
state: mockState, | ||
), | ||
); | ||
} |
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.
try to follow the existing test suite in terms of naming/initializing this widget (e.g. see payout_test.dart)
); | ||
} | ||
|
||
testWidgets('CurrencyDropdown shows the correct initial currency', |
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.
try updating the test descriptions to be consistent with the existing test suite
// Simulate selecting a currency by updating the mockState directly | ||
const selectedCurrency = 'EUR'; | ||
mockState.value = PaymentAmountState( | ||
filterCurrency: | ||
selectedCurrency, // Assuming 'filterCurrency' is the correct field for currency selection | ||
); |
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 updating mockState
directly, we should use tester.tap()
to select a new currency. pull in the latest changes on main to use an updated version of TestData
to make this easier for you to do.
Sure, Thanks for the input. |
testWidgets( | ||
'should update PaymentAmountState when a new offering is selected in CurrencyDropdown', | ||
(tester) async { | ||
await tester.pumpWidget(createWidgetUnderTest()); | ||
await tester.pumpWidget(currencyDropdownTestWidget()); | ||
|
||
// Open the modal by tapping on the currency dropdown button | ||
await tester.tap( | ||
find.text(testCurrency), | ||
); // Using the 'USD' label text to find the button | ||
); | ||
await tester.pumpAndSettle(); | ||
|
||
// Simulate selecting a currency by updating the mockState directly | ||
const selectedCurrency = 'EUR'; | ||
mockState.value = PaymentAmountState( | ||
filterCurrency: | ||
selectedCurrency, // Assuming 'filterCurrency' is the correct field for currency selection | ||
); | ||
await tester.tap(find.text(selectedCurrency)); | ||
|
||
await tester.pumpAndSettle(); | ||
|
||
// Verify the state reflects the selected currency | ||
expect(mockState.value?.filterCurrency, selectedCurrency); | ||
}); |
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.
Hey @ethan-tbd check out this test case. It keeps on failing . How should I tackle it?
testWidgets( | ||
'should update PaymentAmountState when a new offering is selected in CurrencyDropdown', | ||
(tester) async { | ||
await tester.pumpWidget(currencyDropdownTestWidget()); | ||
|
||
await tester.tap( | ||
find.text(testCurrency), | ||
); | ||
await tester.pumpAndSettle(); | ||
|
||
const selectedCurrency = 'EUR'; | ||
await tester.tap(find.text(selectedCurrency)); | ||
|
||
await tester.pumpAndSettle(); | ||
|
||
expect(mockState.value?.filterCurrency, selectedCurrency); | ||
}); |
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 test case fails because you are not initializing offeringsMap
with any offerings that have a payout currency of 'EUR'
. take a look at the optional named parameter in getOfferingsMap()
here: https://github.com/TBD54566975/didpay/blob/main/test/helpers/test_data.dart#L31
late ValueNotifier<PaymentAmountState?> mockState; | ||
const testCurrency = 'USD'; | ||
|
||
setUp(() { | ||
mockState = ValueNotifier<PaymentAmountState?>( | ||
PaymentAmountState( | ||
offeringsMap: TestData.getOfferingsMap(), | ||
), | ||
); | ||
}); |
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.
there's no need to use late
and setUp()
here, you can just create a ValueNotifier
in currencyDropdownTestWidget()
Hey, @ethan-tbd . check out this commit. All tests passed! |
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.
looks good, thanks for making all the suggested changes
This PR introduces several UI tests for the
CurrencyDropdown
widget to verify its functionality and ensure robust state handling.Resolves #287