-
Notifications
You must be signed in to change notification settings - Fork 3
Add setup workflow with error handling and fix AddressFactory Faker instance issue #350
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
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a new GitHub Actions workflow for multi-step setup with granular error handling, updates the workflow documentation to describe the setup process, and refactors the AddressFactory to use Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant MariaDB as MariaDB Service
participant Yarn as Yarn
participant Composer as Composer
participant Env as Environment
participant Artisan as Laravel Artisan
participant Log as Error Log
participant Report as Final Report
GitHub->>MariaDB: Start service
GitHub->>Yarn: Conditional: Run yarn install
Yarn-->>Log: Log errors (if conditional enabled)
GitHub->>Composer: Conditional: Run composer install
Composer-->>Log: Log errors (if conditional enabled)
GitHub->>Env: Conditional: Copy .env.example → .env
Env-->>Log: Log errors (if conditional enabled)
GitHub->>Artisan: Conditional: Generate app key
Artisan-->>Log: Log errors (if conditional enabled)
GitHub->>Artisan: Conditional: Run migrations
Artisan-->>Log: Log errors (if conditional enabled)
GitHub->>Artisan: Conditional: Run seeding
Artisan-->>Log: Log errors (if conditional enabled)
GitHub->>Report: Aggregate results from error log
Report-->>GitHub: Report success or failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…br issue Co-authored-by: nielsdrost7 <[email protected]>
Co-authored-by: nielsdrost7 <[email protected]>
Co-authored-by: nielsdrost7 <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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: 1
🤖 Fix all issues with AI agents
In @.github/workflows/setup.yml:
- Around line 99-106: The "Initialize error tracking" step (id: init) currently
writes a newline into /tmp/setup_errors.log using echo "" >
/tmp/setup_errors.log which makes the later check if [ -s /tmp/setup_errors.log
] report a false positive; change the initialization to create an empty
zero-byte file instead, e.g. use : > /tmp/setup_errors.log or truncate -s 0
/tmp/setup_errors.log (or rm -f /tmp/setup_errors.log && touch
/tmp/setup_errors.log) so the -s test only triggers when real errors are
appended.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/README.md.github/workflows/setup.ymlModules/Clients/Database/Factories/AddressFactory.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use full namespace imports instead of short imports (e.g., use
Illuminate\Support\Facades\LognotLog)
Files:
Modules/Clients/Database/Factories/AddressFactory.php
🧠 Learnings (3)
📚 Learning: 2026-01-08T10:38:10.370Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-08T10:38:10.370Z
Learning: Applies to **/Tests/**/*Export*.php : Export tests must use fakes: `Queue::fake()`, `Storage::fake()`, and verify job dispatching with `Bus::assertChained()`
Applied to files:
Modules/Clients/Database/Factories/AddressFactory.php
📚 Learning: 2026-01-08T10:38:10.369Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-08T10:38:10.369Z
Learning: Applies to **/Tests/**/*.php : Prefer Fakes and Fixtures over Mocks in tests
Applied to files:
Modules/Clients/Database/Factories/AddressFactory.php
📚 Learning: 2026-01-08T10:38:10.370Z
Learnt from: CR
Repo: InvoicePlane/InvoicePlane-v2 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-08T10:38:10.370Z
Learning: Applies to **/Tests/**/*.php : Add type hints when factories return Collection but method expects Model
Applied to files:
Modules/Clients/Database/Factories/AddressFactory.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (6)
Modules/Clients/Database/Factories/AddressFactory.php (1)
28-35: LGTM! Correct fix for the Faker provider issue.The changes correctly replace
fake()with$this->fakerfor all address fields. Since custom Faker providers are added to$this->faker(lines 20-25), using the instance method ensures that custom methods likestateAbbrfrom theen_US\Addressprovider are available. This resolves the "stateAbbr not found" seeding errors mentioned in the PR..github/workflows/setup.yml (4)
1-54: LGTM! Clean workflow structure with well-defined inputs.The workflow uses
workflow_dispatchwith six choice-based boolean inputs for granular step control. Usingtype: choicewith string values'true'/'false'(rather than boolean type) ensures reliable string comparisons in the conditional steps later.
55-81: LGTM! Proper service configuration with health checks.The job configures MariaDB 10.6 with appropriate health checks to ensure the database is ready before the setup steps execute. Database credentials are correctly matched between the service definition and environment variables.
107-280: LGTM! Consistent error handling pattern across all setup steps.All six setup steps follow a consistent and correct pattern:
- Use
set +eto prevent early exit on failure- Capture exit codes for error detection
- Log failures with step name, exit code, and diagnostic hints to
/tmp/setup_errors.log- Set step-specific output flags for the final report
- Always
exit 0to continue to the next stepThe commands use appropriate flags for CI environments (e.g.,
--frozen-lockfilefor Yarn,--no-interactionfor Composer,--forcefor migrations).
281-372: LGTM! Comprehensive final report with clear status indicators.The final report step correctly:
- Uses
if: always()to ensure it runs regardless of previous step outcomes- Aggregates all errors from the log file
- Generates a detailed GitHub Step Summary with visual indicators (✅/❌)
- Exits with status 1 if any errors occurred, otherwise 0
Note: The error detection on line 288 depends on the initialization fix suggested earlier to avoid false positives when no errors occur.
.github/workflows/README.md (1)
406-482: LGTM! Comprehensive and accurate documentation.The new Section 8 documentation accurately describes the setup workflow:
- All six steps match the workflow implementation
- Error handling explanation is correct (uses
set +e, logs to/tmp/setup_errors.log, continues on failure)- Infrastructure details match the workflow configuration (MariaDB 10.6, PHP 8.4, Node.js 22)
- Usage examples provide clear guidance for full setup and selective debugging scenarios
- The step-by-step instructions are easy to follow
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Creates a debuggable GitHub Actions workflow for installation steps with error collection, and fixes database seeding failure caused by incorrect Faker instance usage.
Setup Workflow (
.github/workflows/setup.yml)set +e, logs to/tmp/setup_errors.logEnables debugging specific installation steps by disabling others, while collecting all errors in a single run.
AddressFactory Bug Fix
The factory added custom Faker providers to
$this->faker, then used thefake()helper which creates a fresh instance without those providers:All 8 fields updated to use
$this->fakerconsistently.Documentation
Added workflow documentation (Section 8 in
.github/workflows/README.md) with usage examples and error handling explanation.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
fakerphp.github.io/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit
Documentation
Chores
Note: This release primarily contains internal infrastructure improvements and documentation updates with no direct impact on end-user functionality.
✏️ Tip: You can customize this high-level summary in your review settings.