added Unittests for lib/widgets/event_card.dart #2668
Conversation
sayyad-asifbasha
requested review from
palisadoes and
noman2002
as code owners
WalkthroughThe pull request introduces two new test cases in the Changes
Assessment against linked 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 ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. 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. |
![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 (2)
test/widget_tests/widgets/event_card_test.dart (2)
199-208: LGTM! Consider adding a descriptive doc comment.The test case effectively verifies the creator indicators when the current user is the event creator. The setup and assertions are clear and appropriate.
Consider adding a doc comment to improve test documentation:
+ /// Verifies that when the current user is the event creator: + /// - The verified icon is displayed + /// - The "Created" text is shown testWidgets('Check for Created Row visibility', (tester) async {
210-220: Consider consistency improvements and documentation.A few suggestions to enhance this test case:
- Use
pump()instead ofpumpAndSettle()for consistency with other tests, unless there's a specific need for animations to complete- Consider a more concise name like 'Check Created Row hidden for non-creators'
- Add documentation for clarity
Consider applying these changes:
+ /// Verifies that when the current user is not the event creator: + /// - The verified icon is hidden + /// - The "Created" text is hidden - testWidgets('Check for absence of Created Row for non-creators', + testWidgets('Check Created Row hidden for non-creators', (tester) async { mockNetworkImages(() async { final event = getEvent(); userConfig.currentUser.id = "nonCreatorId"; await tester.pumpWidget(createCustomEventCard(event)); - await tester.pumpAndSettle(); + await tester.pump(); expect(find.byIcon(Icons.verified), findsNothing); expect(find.text('Created'), findsNothing); }); });
| testWidgets('Check for Created Row visibility', (tester) async { | ||
| mockNetworkImages(() async { | ||
| final event = getEvent(); | ||
| userConfig.currentUser.id = event.creator!.id; | ||
| await tester.pumpWidget(createCustomEventCard(event)); | ||
| await tester.pump(); | ||
| expect(find.byIcon(Icons.verified), findsOneWidget); | ||
| expect(find.text('Created'), findsOneWidget); | ||
| }); | ||
| }); | ||
|
|
||
| testWidgets('Check for absence of Created Row for non-creators', | ||
| (tester) async { | ||
| mockNetworkImages(() async { | ||
| final event = getEvent(); | ||
| userConfig.currentUser.id = "nonCreatorId"; | ||
| await tester.pumpWidget(createCustomEventCard(event)); | ||
| await tester.pumpAndSettle(); | ||
| expect(find.byIcon(Icons.verified), findsNothing); | ||
| expect(find.text('Created'), findsNothing); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
💡 Codebase verification
Additional test cases needed for null safety
Looking at the EventCard implementation and existing tests, I can confirm that edge cases for null values need coverage. The widget uses null-aware operators and null checks:
event.isRegistered ?? falseevent.creator!.idis force-unwrapped, which could crash if creator is nullevent.attendees?.length ?? 0has null safety but isn't tested
Add these test cases:
- When event.creator is null
- When event.isRegistered is null (currently defaults to false)
- When event.attendees is null (currently defaults to 0)
🔗 Analysis chain
Verify test coverage for edge cases.
The new tests effectively cover the basic creator visibility scenarios. Consider adding tests for these edge cases:
- When creator field is null
- When current user ID is null
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these edge cases are already covered elsewhere
rg -A 5 "creator.*null|currentUser.*null" "test/widget_tests"
Length of output: 61
Script:
#!/bin/bash
# Let's check the event_card_test.dart file to understand the test setup and existing test cases
cat test/widget_tests/widgets/event_card_test.dart
Length of output: 6706
Script:
#!/bin/bash
# Let's check the EventCard widget implementation to understand how it handles null cases
cat talawa/widgets/event_card.dart
Length of output: 99
Script:
#!/bin/bash
# Let's find the correct path to the event_card widget
fd -t f "event_card.dart"
Length of output: 53
Script:
#!/bin/bash
# Now let's check the EventCard widget implementation
cat lib/widgets/event_card.dart
Length of output: 12605
| name: CI Workflow | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: | ||
| branches: [main] | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| # Step 1: Checkout your repository | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| # Step 2: Set up Node.js | ||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' # Adjust Node.js version if needed | ||
|
|
||
| # Step 3: Install project dependencies | ||
| - name: Install dependencies | ||
| run: npm install | ||
|
|
||
| # Step 4: Run tests and generate coverage report | ||
| - name: Run tests with Jest | ||
| run: npx jest --coverage | ||
|
|
||
| # Step 5: Upload coverage report to Codecov | ||
| - name: Upload coverage reports to Codecov | ||
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| slug: sayyad-asifbasha/mess-backend-firebase |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide complete Flutter CI workflow template
Here's a complete Flutter CI workflow template that would be more appropriate for this project:
name: Flutter CI
on:
push:
branches: [main]
pull_request:
branches: [main]
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: Set up Flutter
uses: subosito/flutter-action@v2
with:
flutter-version: '3.x'
channel: 'stable'
- name: Install dependencies
run: flutter pub get
- name: Analyze code
run: flutter analyze
- name: Run tests
run: flutter test --coverage
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
slug: PalisadoesFoundation/talawaThis workflow includes:
- Proper Flutter SDK setup
- Flutter dependency installation
- Static code analysis
- Flutter test runner with coverage
- Correct repository configuration for Codecov
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' # Adjust Node.js version if needed | ||
|
|
||
| # Step 3: Install project dependencies | ||
| - name: Install dependencies | ||
| run: npm install |
There was a problem hiding this comment.
Critical: Incorrect technology stack configuration
This workflow is configured for Node.js/Jest testing, but the project is a Flutter/Dart application. The Node.js setup and npm dependencies are not appropriate for testing Flutter widgets.
Replace the Node.js setup with Flutter SDK setup:
- # Step 2: Set up Node.js
- - name: Set up Node.js
- uses: actions/setup-node@v4
- with:
- node-version: '20' # Adjust Node.js version if needed
-
- # Step 3: Install project dependencies
- - name: Install dependencies
- run: npm install
+ # Step 2: Set up Flutter
+ - name: Set up Flutter
+ uses: subosito/flutter-action@v2
+ with:
+ flutter-version: '3.x'
+ channel: 'stable'
+
+ # Step 3: Get Flutter dependencies
+ - name: Get dependencies
+ run: flutter pub get📝 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.
| - name: Set up Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '20' # Adjust Node.js version if needed | |
| # Step 3: Install project dependencies | |
| - name: Install dependencies | |
| run: npm install | |
| - name: Set up Flutter | |
| uses: subosito/flutter-action@v2 | |
| with: | |
| flutter-version: '3.x' | |
| channel: 'stable' | |
| # Step 3: Get Flutter dependencies | |
| - name: Get dependencies | |
| run: flutter pub get |
| - name: Run tests with Jest | ||
| run: npx jest --coverage |
There was a problem hiding this comment.
Critical: Incorrect test runner configuration
The workflow is using Jest for testing, which is a JavaScript test runner. Flutter tests should be run using flutter test.
Replace the Jest test command with Flutter test:
- # Step 4: Run tests and generate coverage report
- - name: Run tests with Jest
- run: npx jest --coverage
+ # Step 4: Run Flutter tests with coverage
+ - name: Run tests
+ run: flutter test --coverage📝 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.
| - name: Run tests with Jest | |
| run: npx jest --coverage | |
| # Step 4: Run Flutter tests with coverage | |
| - name: Run tests | |
| run: flutter test --coverage |
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| slug: sayyad-asifbasha/mess-backend-firebase |
There was a problem hiding this comment.
Critical: Incorrect repository configuration
The Codecov slug is pointing to sayyad-asifbasha/mess-backend-firebase instead of PalisadoesFoundation/talawa.
Fix the repository configuration:
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
- slug: sayyad-asifbasha/mess-backend-firebase
+ slug: PalisadoesFoundation/talawa📝 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.
| token: ${{ secrets.CODECOV_TOKEN }} | |
| slug: sayyad-asifbasha/mess-backend-firebase | |
| token: ${{ secrets.CODECOV_TOKEN }} | |
| slug: PalisadoesFoundation/talawa |
|
We have a policy of unassigning contributors who close PRs without getting validation from our reviewer team. This is because:
Please be considerate of our volunteers' limited time and our desire to improve our code base. This policy is stated as a pinned post in all our Talawa repositories. Our YouTube videos explain why this practice is not acceptable to our Community. Unfortunately, if this continues we will have to close the offending PR and unassign you from the issue. |
What kind of change does this PR introduce?
Unittests for lib/widgets/event_card.dart
Issue Number:
Fixes #2618
Did you add tests for your changes?
Yes
Snapshots/Videos:
Have you read the contributing guide?
Yes
Summary by CodeRabbit