CI #3890
CI #3890
Conversation
|
Warning Rate limit exceeded@rrousselGit has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces modifications to the GitHub Actions workflow configuration file Changes
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
Line range hint
1-116: Document CI changes and create a restoration planThe current changes appear to be temporary debugging modifications to the CI workflow. To maintain good CI/CD practices:
- Add a comment at the top of the file explaining:
- Why these changes were made
- Which packages are temporarily excluded
- Link to any relevant issues
- Create a timeline for restoring full CI coverage
- Consider splitting the workflow into multiple jobs to isolate problematic packages
Here's a suggested header comment:
name: Build +# Temporary CI configuration +# TODO: Restore full package coverage (tracking issue: #XXXX) +# - Currently testing only riverpod_generator due to CI setup issues +# - Other packages temporarily disabled while investigating build failures +# - Expected restoration date: YYYY-MM-DD + on: pull_request:
.github/workflows/build.yml
Outdated
| - name: Check | ||
| run: ls -la . | ||
|
|
||
| - name: Check2 | ||
| run: | | ||
| if test -d "integration/build_yaml"; then | ||
| flutter pub get integration/build_yaml | ||
| fi | ||
|
|
There was a problem hiding this comment.
Remove redundant diagnostic steps
The new diagnostic steps appear to be temporary debugging additions:
- The "Check" step (ls -la) seems to be for debugging purposes
- The "Check2" step is an exact duplicate of the existing "Install dependencies (integration/buid.yaml)" step
Remove these redundant steps as they:
- Increase workflow execution time
- Add maintenance overhead
- Don't provide additional value
- - name: Check
- run: ls -la .
-
- - name: Check2
- run: |
- if test -d "integration/build_yaml"; then
- flutter pub get integration/build_yaml
- fi
-
.github/workflows/build.yml
Outdated
| # - stable | ||
| - master | ||
| package_path: | ||
| - examples/counter | ||
| - examples/marvel | ||
| - examples/pub | ||
| - examples/random_number | ||
| - examples/stackoverflow | ||
| - examples/todos | ||
| - packages/riverpod | ||
| - packages/riverpod/example | ||
| - packages/flutter_riverpod | ||
| - packages/flutter_riverpod/example | ||
| - packages/hooks_riverpod | ||
| - packages/hooks_riverpod/example | ||
| - packages/riverpod_annotation | ||
| # TODO(rrousselGit) update riverpod_cli test setup to be supported by the CI | ||
| # - packages/riverpod_cli | ||
| # - examples/counter | ||
| # - examples/marvel | ||
| # - examples/pub | ||
| # - examples/random_number | ||
| # - examples/stackoverflow | ||
| # - examples/todos | ||
| # - packages/riverpod | ||
| # - packages/riverpod/example | ||
| # - packages/flutter_riverpod | ||
| # - packages/flutter_riverpod/example | ||
| # - packages/hooks_riverpod | ||
| # - packages/hooks_riverpod/example | ||
| # - packages/riverpod_annotation | ||
| # # TODO(rrousselGit) update riverpod_cli test setup to be supported by the CI | ||
| # # - packages/riverpod_cli | ||
| - packages/riverpod_generator | ||
| - packages/riverpod_generator/integration/build_yaml | ||
| # TODO(rrousselGit) update riverpod_graph test setup to be supported by the CI | ||
| # - packages/riverpod_graph | ||
| # TODO(rousselGit) blocked by https://github.com/invertase/dart_custom_lint/issues/16 | ||
| # - packages/riverpod_lint_flutter_test | ||
| - website | ||
| # - website |
There was a problem hiding this comment.
💡 Codebase verification
Critical test coverage reduction needs justification
The verification reveals significant concerns about the CI changes:
- All core packages (
riverpod,flutter_riverpod,hooks_riverpod) have extensive test suites with 50+ test files but are currently excluded from CI. - These are actively maintained packages at version 2.6.1/2.6.3, not deprecated or experimental ones.
- The TODOs in the CI configuration indicate known issues:
riverpod_clineeds CI setup updatesriverpod_graphhas pending test setup changesriverpod_lint_flutter_testis blocked by an external issue
While there might be valid reasons for temporarily reducing the test matrix, the current changes:
- Remove testing for core packages that have extensive test coverage
- Skip the stable channel testing entirely
- Don't provide clear documentation about why these changes were made
Recommendations:
- Add a comment explaining why these packages were excluded and whether this is temporary
- Consider keeping at least the core packages (
riverpod,flutter_riverpod,hooks_riverpod) in CI - Create tracking issues for the TODO items if they don't exist already
- Keep the stable channel testing to ensure compatibility isn't broken
🔗 Analysis chain
Verify the impact of reduced test matrix coverage
The significant reduction in the test matrix raises several concerns:
- Testing only on
masterchannel while commenting outstablecould miss compatibility issues - Most packages are excluded from CI, which could lead to undetected regressions
- Several TODOs indicate known CI setup issues for specific packages
Let's check which packages actually need testing:
Consider:
- Adding a comment explaining why these changes were made and their temporary nature
- Creating tracking issues for the TODO items to ensure they're addressed
- Implementing a phased approach to fix CI issues while maintaining some level of test coverage
Significant test coverage gaps detected in CI configuration
The verification results show that many packages with test directories are currently excluded from CI:
- Core packages with tests:
flutter_riverpod,hooks_riverpod,riverpod - Example apps with tests:
counter,todos - Tools with tests:
riverpod_graph,riverpod_lint_flutter_test
Let's verify if these packages are actively maintained and if their tests are currently passing:
Let's try a different approach to check the package activity and test status:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify which packages contain testable code
# Look for packages with test directories to determine which ones should be in CI
echo "Packages with test directories:"
fd -t d '^test$' packages/ examples/
echo -e "\nPackages with pubspec.yaml:"
fd pubspec.yaml packages/ examples/
Length of output: 1644
Script:
#!/bin/bash
# Check the last modification dates of test files in these packages
echo "Last modified test files in each package:"
for pkg in flutter_riverpod hooks_riverpod riverpod riverpod_graph riverpod_lint_flutter_test; do
echo -e "\n=== $pkg ==="
git log -1 --format="%ad" -- "packages/$pkg/test/"
done
# Check if there are any recent changes to these packages
echo -e "\nRecent changes to package directories:"
for pkg in flutter_riverpod hooks_riverpod riverpod riverpod_graph riverpod_lint_flutter_test; do
echo -e "\n=== $pkg ==="
git log -1 --format="%ad" -- "packages/$pkg/"
done
Length of output: 1787
Script:
#!/bin/bash
# Check for test files and their content in key packages
echo "=== Core packages test files ==="
fd -t f '.*_test\.dart$' packages/riverpod/test packages/flutter_riverpod/test packages/hooks_riverpod/test
echo -e "\n=== Package versions from pubspec.yaml ==="
for pkg in riverpod flutter_riverpod hooks_riverpod riverpod_generator; do
echo -e "\n$pkg:"
rg "version:" "packages/$pkg/pubspec.yaml" --no-line-number || true
done
echo -e "\n=== CI configuration comments ==="
rg "^\\s*#.*TODO" .github/workflows/build.yml
Length of output: 5567
Related Issues
fixes #your-issue-number
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).I have updated the
CHANGELOG.mdof the relevant packages.Changelog files must be edited under the form:
If this contains new features or behavior changes,
I have updated the documentation to match those changes.
Summary by CodeRabbit