-
Notifications
You must be signed in to change notification settings - Fork 1
Add code style checks and PHPUnit configuration for testing #5
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAktualisierung des Code-Style-Workflows auf actions/checkout@v5, Hinzufügen eines Test-Workflows für PHP 8.2–8.4, Ergänzung von Composer um phpunit und Autoloading für Tests, neue phpunit.xml-Konfiguration und ein Feature-Test für das Logging mittels Service Provider und LogManager. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Job as CI Job (Matrix: PHP 8.2/8.3/8.4)
Dev->>GH: push
GH->>Job: Start "Execute tests"
Job->>Job: actions/checkout@v5
Job->>Job: setup-php@v2 (matrix PHP)
Job->>Job: composer install
Job->>Job: composer test (phpunit)
sequenceDiagram
autonumber
participant PU as PHPUnit
participant VA as vendor/autoload.php
participant TB as Orchestra Testbench
participant SP as ErrorLoggerServiceProvider
participant LM as LogManager
participant ML as Monolog
PU->>VA: Bootstrap
PU->>TB: Starte TestCase
TB->>SP: Provider registrieren
PU->>LM: Log Debug/Error
LM->>ML: Delegiere Logeintrag
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 Minuten Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (12)
.github/workflows/code-style.yml (1)
21-24: Composer-Cache aktivieren, um CI-Laufzeiten deutlich zu reduzierenDas Setup-Action unterstützt integriertes Composer-Caching. Bitte aktivieren:
Wende diesen Diff an:
- name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} + cache: composer.github/workflows/tests.yml (5)
6-6: Job-ID ist irreführend benanntDer Workflow heißt „Execute tests“, der Job-Key jedoch „code-style“. Benenne den Job-Key passend (z. B. „tests“) für Klarheit in den Build-Logs.
Wende diesen Diff an:
- code-style: + tests:
28-29: Step-Name stimmt nicht mit der Aktion übereinDer Step heißt „Code style checks for PHP“, führt aber Tests aus. Bitte umbenennen.
Wende diesen Diff an:
- - name: Code style checks for PHP + - name: Run tests run: composer test
12-12: YAML: Leerzeichen nach Kommata im Matrix-Eintrag hinzufügenYAMLlint warnt vor fehlenden Leerzeichen. Korrigiert die Lesbarkeit (keine funktionale Änderung).
Wende diesen Diff an:
- php: [8.2,8.3,8.4] + php: [8.2, 8.3, 8.4]
21-24: Composer-Cache aktivieren, um Testläufe zu beschleunigenWie im Code-Style-Workflow empfohlen: Composer-Caching spart spürbar Zeit.
Wende diesen Diff an:
- name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} + cache: composer
29-29: Kein Newline am DateiendeYAMLlint meldet „no new line character at the end of file“. Füge eine abschließende Leerzeile hinzu.
composer.json (2)
40-40: PHP-Versionsconstraint vereinfachen„^8.2|^8.3|^8.4“ ist redundant, da „^8.2“ bereits alle 8.x abdeckt (bis <9.0). Vereinfacht die Angabe ohne Funktionsänderung.
Wende diesen Diff an:
- "php": "^8.2|^8.3|^8.4", + "php": "^8.2",
43-49: Optionale Stilangleichung: OR-Operator „||“ statt „|“Composer akzeptiert „|“ und „||“. Viele Projekte bevorzugen „||“ wegen Lesbarkeit. Kein Muss – rein stilistisch.
Wende diesen Diff an:
- "illuminate/log": "^10|^11|^12", - "illuminate/mail": "^10|^11|^12", - "illuminate/support": "^10|^11|^12", + "illuminate/log": "^10 || ^11 || ^12", + "illuminate/mail": "^10 || ^11 || ^12", + "illuminate/support": "^10 || ^11 || ^12", @@ - "m4tthumphrey/php-gitlab-api": "^11.14|^12.0.0", + "m4tthumphrey/php-gitlab-api": "^11.14 || ^12.0.0",tests/Feature/ErrorLoggerTest.php (4)
12-15: PHPMD-Warnung unterdrücken: unbenutzter Parameter $appTestbench verlangt die Signatur; PHPMD meldet sie dennoch. Unterdrücke die Warnung auf Methodenebene.
Wende diesen Diff an:
- protected function getPackageProviders($app): array - { - return [ErrorLoggerServiceProvider::class]; - } + /** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + protected function getPackageProviders($app): array + { + return [ErrorLoggerServiceProvider::class]; + }
19-22: Logger aus dem Container beziehen und Schein-Assertion entfernen
- Nutze den Container, um Konfiguration/Bindings zu respektieren.
- Verwende expectNotToPerformAssertions(), statt assertTrue(true).
Wende diesen Diff an:
- $logger = new LogManager($this->app); - - $logger->log(Level::Debug, 'message'); - $this->assertTrue(true); + $logger = $this->app->make(LogManager::class); + $this->expectNotToPerformAssertions(); + + $logger->log(Level::Debug, 'message');
27-30: Gleiches Refactoring für den zweiten TestContainer nutzen und Schein-Assertion entfernen.
Wende diesen Diff an:
- $logger = new LogManager($this->app); - - $logger->log(Level::Error, 'message'); - $this->assertTrue(true); + $logger = $this->app->make(LogManager::class); + $this->expectNotToPerformAssertions(); + + $logger->log(Level::Error, 'message');
17-31: Optional: Duplizierung vermeiden via Data ProviderBeide Tests sind identisch bis auf das Level. Ein Data Provider reduziert Duplizierung und macht Erweiterungen einfacher.
Auf Wunsch liefere ich dir eine Variante mit Data Provider (Docblock oder Attribute) für PHPUnit 11.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/code-style.yml(1 hunks).github/workflows/tests.yml(1 hunks)composer.json(1 hunks)phpunit.xml(1 hunks)tests/Feature/ErrorLoggerTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/Feature/ErrorLoggerTest.php (2)
src/ErrorLoggerServiceProvider.php (1)
ErrorLoggerServiceProvider(9-29)src/Logging/LogManager.php (1)
LogManager(12-137)
🪛 YAMLlint (1.37.1)
.github/workflows/tests.yml
[warning] 12-12: too few spaces after comma
(commas)
[warning] 12-12: too few spaces after comma
(commas)
[error] 29-29: no new line character at the end of file
(new-line-at-end-of-file)
🪛 PHPMD (2.15.0)
tests/Feature/ErrorLoggerTest.php
12-12: Avoid unused parameters such as '$app'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (3)
.github/workflows/code-style.yml (1)
18-18: Upgrade auf actions/checkout@v5 sieht gut ausDie Aktualisierung ist konsistent mit dem neuen Test-Workflow und unproblematisch.
phpunit.xml (1)
3-12: PHPUnit-Konfiguration ist solide und kompatibel mit dem Setup
- Testsuite „Feature“ passt zu eurer Teststruktur.
- Bootstrap via vendor/autoload.php ist korrekt.
- Source-Include für ./src ist sinnvoll für Coverage/Filterung.
composer.json (1)
69-73: Composer-Skripte sauber integriertDas neue „test“-Skript harmoniert mit dem CI-Workflow („composer test“). Passt.
Summary by CodeRabbit
Tests
Chores