-
-
Notifications
You must be signed in to change notification settings - Fork 498
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
Test: added test for Organization search list #2644
Test: added test for Organization search list #2644
Conversation
WalkthroughThis pull request introduces a comprehensive suite of widget tests for the Changes
Assessment against linked issues
Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if either of these two conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
Other🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise. |
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: 5
🧹 Outside diff range and nitpick comments (1)
test/widget_tests/widgets/organization_search_list_test.dart (1)
22-25
: Add tearDown to clean up resourcesWhile the setUp is properly implemented, it's a good practice to include a tearDown to clean up any resources and reset the state between tests.
Add after the setUp block:
tearDown(() { reset(mockModel); reset(mockGraphQLClient); });
testWidgets('Shows error message and attempts refetch on exception', | ||
(WidgetTester tester) async { | ||
when(mockModel.searchController.text).thenReturn('test'); | ||
when(mockGraphQLClient.query( | ||
QueryOptions( | ||
document: gql(queries.getPluginsList()), | ||
), | ||
)).thenAnswer((_) async { | ||
throw Exception('GraphQL error'); | ||
}); | ||
|
||
await tester.pumpWidget(createWidgetUnderTest()); | ||
await tester.pump(); | ||
|
||
// Expect error handling and retry logic here | ||
expect(find.textContaining('GraphQL error'), findsNothing); | ||
}); |
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
Improve error handling test coverage
The current error handling test doesn't fully verify the error state and recovery flow:
- It only checks that the error message is not immediately shown
- It doesn't verify the retry mechanism
Enhance the test with proper error state verification:
testWidgets('Shows error message and attempts refetch on exception',
(WidgetTester tester) async {
when(mockModel.searchController.text).thenReturn('test');
when(mockGraphQLClient.query(any)).thenThrow(Exception('GraphQL error'));
await tester.pumpWidget(createWidgetUnderTest());
await tester.pumpAndSettle(); // Wait for error state
// Verify error message is shown
expect(find.textContaining('Error'), findsOneWidget);
// Verify retry button exists
final retryButton = find.byIcon(Icons.refresh);
expect(retryButton, findsOneWidget);
// Test retry mechanism
await tester.tap(retryButton);
await tester.pump();
// Verify query was called again
verify(mockGraphQLClient.query(any)).called(2);
});
testWidgets('Calls fetchMoreHelper when near end of list', | ||
(WidgetTester tester) async { | ||
final organizations = List<OrgInfo>.generate( | ||
30, (index) => OrgInfo(name: 'Org $index', id: '$index')); | ||
|
||
when(mockModel.searchController.text).thenReturn('org'); | ||
when(mockModel.organizations).thenReturn(organizations); | ||
|
||
await tester.pumpWidget(createWidgetUnderTest()); | ||
|
||
await tester.scrollUntilVisible( | ||
find.byType(PageView).first, | ||
500.0, | ||
); | ||
Future<QueryResult> Function(FetchMoreOptions)? fetchMore; | ||
verify(mockModel.fetchMoreHelper(fetchMore!, organizations)).called(1); | ||
}); |
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.
Fix type safety issue in fetchMore test
The fetchMore test has potential type safety issues and doesn't properly mock the fetchMore callback.
Improve the test implementation:
testWidgets('Calls fetchMoreHelper when near end of list',
(WidgetTester tester) async {
final organizations = List<OrgInfo>.generate(
30, (index) => OrgInfo(name: 'Org $index', id: '$index'));
final mockFetchMore = MockFetchMoreFunction();
when(mockModel.searchController.text).thenReturn('org');
when(mockModel.organizations).thenReturn(organizations);
await tester.pumpWidget(createWidgetUnderTest());
await tester.scrollUntilVisible(
find.byType(CustomListTile).last,
500.0,
);
verify(mockModel.fetchMoreHelper(mockFetchMore, organizations)).called(1);
});
class MockFetchMoreFunction extends Mock {
Future<QueryResult> call(FetchMoreOptions options) async {
return QueryResult(source: QueryResultSource.network, data: {});
}
}
import 'package:graphql_flutter/graphql_flutter.dart'; | ||
import 'package:mockito/mockito.dart'; | ||
import 'package:talawa/models/organization/org_info.dart'; | ||
import 'package:talawa/view_model/pre_auth_view_models/select_organization_view_model.dart'; | ||
import 'package:talawa/widgets/custom_list_tile.dart'; | ||
import 'package:talawa/widgets/organization_search_list.dart'; | ||
|
||
import '../../helpers/test_locator.dart'; | ||
|
||
class MockSelectOrganizationViewModel extends Mock | ||
implements SelectOrganizationViewModel {} | ||
|
||
class MockGraphQLClient extends Mock implements GraphQLClient {} | ||
|
||
void main() { | ||
late MockSelectOrganizationViewModel mockModel; | ||
late MockGraphQLClient mockGraphQLClient; | ||
|
||
setUp(() { | ||
mockModel = MockSelectOrganizationViewModel(); | ||
mockGraphQLClient = MockGraphQLClient(); | ||
}); | ||
|
||
Widget createWidgetUnderTest() { | ||
return MaterialApp( | ||
home: Scaffold( | ||
body: OrganizationSearchList(model: mockModel), | ||
), | ||
); | ||
} | ||
|
||
testWidgets('OrganizationSearchList renders correctly', | ||
(WidgetTester tester) async { | ||
when(mockModel.searchController.text).thenReturn(''); | ||
await tester.pumpWidget(createWidgetUnderTest()); | ||
expect(find.byType(Query), findsOneWidget); | ||
}); | ||
|
||
testWidgets('Displays loading indicator when fetching data', | ||
(WidgetTester tester) async { | ||
when(mockModel.searchController.text).thenReturn('test'); | ||
await tester.pumpWidget(createWidgetUnderTest()); | ||
|
||
// Simulate loading state | ||
await tester.pump(); | ||
expect(find.byType(CupertinoActivityIndicator), findsWidgets); | ||
}); | ||
|
||
testWidgets('Displays CustomListTile for each organization item', | ||
(WidgetTester tester) async { | ||
final organizations = [ | ||
OrgInfo(name: 'Org1', id: '1'), | ||
OrgInfo(name: 'Org2', id: '2'), | ||
]; | ||
|
||
when(mockModel.searchController.text).thenReturn('org'); | ||
when(mockModel.organizations).thenReturn(organizations); | ||
|
||
await tester.pumpWidget(createWidgetUnderTest()); | ||
await tester.pump(); | ||
|
||
expect(find.byType(CustomListTile), findsNWidgets(2)); | ||
}); | ||
|
||
testWidgets('Shows error message and attempts refetch on exception', | ||
(WidgetTester tester) async { | ||
when(mockModel.searchController.text).thenReturn('test'); | ||
when(mockGraphQLClient.query( | ||
QueryOptions( | ||
document: gql(queries.getPluginsList()), | ||
), | ||
)).thenAnswer((_) async { | ||
throw Exception('GraphQL error'); | ||
}); | ||
|
||
await tester.pumpWidget(createWidgetUnderTest()); | ||
await tester.pump(); | ||
|
||
// Expect error handling and retry logic here | ||
expect(find.textContaining('GraphQL error'), findsNothing); | ||
}); | ||
|
||
testWidgets('Calls fetchMoreHelper when near end of list', | ||
(WidgetTester tester) async { | ||
final organizations = List<OrgInfo>.generate( | ||
30, (index) => OrgInfo(name: 'Org $index', id: '$index')); | ||
|
||
when(mockModel.searchController.text).thenReturn('org'); | ||
when(mockModel.organizations).thenReturn(organizations); | ||
|
||
await tester.pumpWidget(createWidgetUnderTest()); | ||
|
||
await tester.scrollUntilVisible( | ||
find.byType(PageView).first, | ||
500.0, | ||
); | ||
Future<QueryResult> Function(FetchMoreOptions)? fetchMore; | ||
verify(mockModel.fetchMoreHelper(fetchMore!, organizations)).called(1); | ||
}); | ||
} |
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
Add missing test scenarios for complete coverage
To achieve 100% code coverage as per PR objectives, the following test scenarios are missing:
- Search input changes
- Organization selection interaction
- Empty state handling
- Network connectivity issues
Would you like me to provide implementations for these additional test scenarios?
💡 Codebase verification
Test coverage needs improvement for error handling and pagination scenarios
The current test suite has gaps in coverage for the following scenarios:
- Error handling with refetch attempts (noOfRefetch <= maxRefetch logic)
- GraphQL result processing when data is loaded (!result.isLoading branch)
- Visibility detection behavior for pagination
- Separator builder implementation
Required test cases:
- Test maximum refetch attempts (currently set to 10)
- Verify organization data parsing from GraphQL response
- Test visibility detector callback with different visibility fractions
- Verify separator rendering with correct padding
🔗 Analysis chain
Verify test coverage meets PR requirements
The PR objectives require 100% code coverage for the organization_search_list.dart
file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage for organization_search_list.dart
# Find the coverage file
fd -t f "lcov.info" -x cat {} | grep -A 1 "organization_search_list.dart"
# Find untested methods in the source file
ast-grep --pattern 'class OrganizationSearchList {
$$$
$method($_) {
$$$
}
$$$
}'
Length of output: 169
Script:
#!/bin/bash
# Let's first locate and examine the widget implementation
fd -t f "organization_search_list.dart" -x cat {}
# Then find all test files related to this widget
fd -t f "organization_search_list.*test.dart" -x cat {}
Length of output: 9171
when(mockGraphQLClient.query( | ||
QueryOptions( | ||
document: gql(queries.getPluginsList()), | ||
), |
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.
Fix undefined queries
reference
The code references queries.getPluginsList()
but there's no import for the queries file containing this function.
Add the missing import at the top of the file:
+import 'package:talawa/services/queries.dart'; // Adjust the import path as needed
Committable suggestion skipped: line range outside the PR's diff.
Make sure that:
|
Closing this as this issue is already resolved in this #2646 |
What kind of change does this PR introduce?
Issue Number:
Fixes #2612
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
This code adds tests for
lib/widgets/organization_search_list.dart
intest/widget_tests/widgets/organization_search_list_test.dart
Have you read the contributing guide?
Summary by CodeRabbit
OrganizationSearchList
component to ensure proper rendering, loading states, error handling, and pagination functionality.