feat: add unit tests#670
Conversation
|
Warning Review limit reached
More reviews will be available in 25 minutes and 12 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR introduces comprehensive test infrastructure for the SimpleCalendar project, including PHPUnit configuration, a WordPress-compatible base test class using Brain Monkey for function stubbing, test bootstrap setup, and unit test suites for the Event class, shared utility functions, and Objects initialization logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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 (2)
.github/workflows/build.yml (1)
21-22: Prefer the Composer script as the CI test entrypoint.Using
composer testhere keeps local and CI execution paths aligned withcomposer.json.Suggested tweak
- name: Run unit tests - run: vendor/bin/phpunit + run: composer test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 21 - 22, Replace the direct PHPUnit invocation in the "Run unit tests" step (currently running vendor/bin/phpunit) with the Composer script entrypoint by calling composer test so CI uses the same test command as local runs; update the run command from vendor/bin/phpunit to composer test and ensure the composer.json "test" script exists and invokes phpunit (adjust the step name only if desired).tests/unit/Events/EventTest.php (1)
23-27: Use fixed timestamps in defaults for deterministic fixtures.Using multiple
Carbon::now()calls in Line 23-Line 27 can introduce subtle, time-dependent fixture variance. A fixed epoch in the default payload keeps tests fully repeatable.Suggested tweak
- 'start' => Carbon::now('UTC')->getTimestamp(), - 'start_utc' => Carbon::now('UTC')->getTimestamp(), + 'start' => 1700000000, + 'start_utc' => 1700000000, 'start_timezone' => 'America/New_York', - 'end' => Carbon::now('UTC')->addHour()->getTimestamp(), - 'end_utc' => Carbon::now('UTC')->addHour()->getTimestamp(), + 'end' => 1700003600, + 'end_utc' => 1700003600,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/Events/EventTest.php` around lines 23 - 27, The default fixture uses multiple Carbon::now() calls for the 'start', 'start_utc', 'start_timezone', 'end', and 'end_utc' keys in EventTest which makes tests time-dependent; replace the dynamic Carbon::now() calls with a single fixed epoch (e.g. a constant timestamp or a fixed Carbon created from a literal) and compute both start and end from that fixed value (end = start + 3600) so the payload in EventTest is deterministic and repeatable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 15-20: The CI step "Create guzzle function stubs" is overwriting
files under vendor/, which hides real dependency/autoload issues; instead stop
writing into vendor/ and place any test stubs under the repository (e.g.,
tests/mocks or phpunit bootstrap) and load them via composer autoload-dev or
your test bootstrap. Remove the commands that create vendor/guzzlehttp/... files
from the workflow, create the stub file(s) in a non-vendor path (referenced as
tests/mocks/functions_include.php), and ensure composer.json's autoload-dev or
your test bootstrap requires that path so tests run without modifying vendor.
Use this change to preserve real install behavior and surface missing
dependency/autoload problems.
In `@tests/bootstrap.php`:
- Line 3: The tests/bootstrap.php file unconditionally defines the ABSPATH
constant which can trigger "already defined" warnings; wrap the definition in a
guard that checks defined('ABSPATH') before calling define so ABSPATH is only
set when not already present (replace the current define('ABSPATH',
'/tmp/fake-wp/'); with an if (!defined('ABSPATH')) { define('ABSPATH',
'/tmp/fake-wp/'); } pattern to avoid redefinition warnings).
---
Nitpick comments:
In @.github/workflows/build.yml:
- Around line 21-22: Replace the direct PHPUnit invocation in the "Run unit
tests" step (currently running vendor/bin/phpunit) with the Composer script
entrypoint by calling composer test so CI uses the same test command as local
runs; update the run command from vendor/bin/phpunit to composer test and ensure
the composer.json "test" script exists and invokes phpunit (adjust the step name
only if desired).
In `@tests/unit/Events/EventTest.php`:
- Around line 23-27: The default fixture uses multiple Carbon::now() calls for
the 'start', 'start_utc', 'start_timezone', 'end', and 'end_utc' keys in
EventTest which makes tests time-dependent; replace the dynamic Carbon::now()
calls with a single fixed epoch (e.g. a constant timestamp or a fixed Carbon
created from a literal) and compute both start and end from that fixed value
(end = start + 3600) so the payload in EventTest is deterministic and
repeatable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b87c3bb5-3a43-43be-85e8-d7c518234e86
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/build.yml.gitignorecomposer.jsontests/bootstrap.phptests/unit/Events/EventTest.phptests/unit/Functions/SharedFunctionsTest.phptests/unit/ObjectsTest.phptests/unit/TestCase.php
| - name: Create guzzle function stubs | ||
| run: | | ||
| mkdir -p vendor/guzzlehttp/guzzle/src | ||
| echo '<?php' > vendor/guzzlehttp/guzzle/src/functions_include.php | ||
| mkdir -p vendor/guzzlehttp/promises/src | ||
| echo '<?php' > vendor/guzzlehttp/promises/src/functions_include.php |
There was a problem hiding this comment.
Avoid overwriting dependency files in CI setup.
Lines 17-20 currently overwrite files in vendor/, which can mask autoload/dependency problems and make test behavior diverge from real installs.
Suggested safe change
- name: Create guzzle function stubs
run: |
mkdir -p vendor/guzzlehttp/guzzle/src
- echo '<?php' > vendor/guzzlehttp/guzzle/src/functions_include.php
+ [ -f vendor/guzzlehttp/guzzle/src/functions_include.php ] || printf '%s\n' '<?php' > vendor/guzzlehttp/guzzle/src/functions_include.php
mkdir -p vendor/guzzlehttp/promises/src
- echo '<?php' > vendor/guzzlehttp/promises/src/functions_include.php
+ [ -f vendor/guzzlehttp/promises/src/functions_include.php ] || printf '%s\n' '<?php' > vendor/guzzlehttp/promises/src/functions_include.php📝 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: Create guzzle function stubs | |
| run: | | |
| mkdir -p vendor/guzzlehttp/guzzle/src | |
| echo '<?php' > vendor/guzzlehttp/guzzle/src/functions_include.php | |
| mkdir -p vendor/guzzlehttp/promises/src | |
| echo '<?php' > vendor/guzzlehttp/promises/src/functions_include.php | |
| - name: Create guzzle function stubs | |
| run: | | |
| mkdir -p vendor/guzzlehttp/guzzle/src | |
| [ -f vendor/guzzlehttp/guzzle/src/functions_include.php ] || printf '%s\n' '<?php' > vendor/guzzlehttp/guzzle/src/functions_include.php | |
| mkdir -p vendor/guzzlehttp/promises/src | |
| [ -f vendor/guzzlehttp/promises/src/functions_include.php ] || printf '%s\n' '<?php' > vendor/guzzlehttp/promises/src/functions_include.php |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build.yml around lines 15 - 20, The CI step "Create guzzle
function stubs" is overwriting files under vendor/, which hides real
dependency/autoload issues; instead stop writing into vendor/ and place any test
stubs under the repository (e.g., tests/mocks or phpunit bootstrap) and load
them via composer autoload-dev or your test bootstrap. Remove the commands that
create vendor/guzzlehttp/... files from the workflow, create the stub file(s) in
a non-vendor path (referenced as tests/mocks/functions_include.php), and ensure
composer.json's autoload-dev or your test bootstrap requires that path so tests
run without modifying vendor. Use this change to preserve real install behavior
and surface missing dependency/autoload problems.
| @@ -0,0 +1,28 @@ | |||
| <?php | |||
|
|
|||
| define('ABSPATH', '/tmp/fake-wp/'); | |||
There was a problem hiding this comment.
Guard ABSPATH before defining it.
Line 3 can raise “already defined” warnings in shared/embedded test runners.
Suggested fix
-define('ABSPATH', '/tmp/fake-wp/');
+if (!defined('ABSPATH')) {
+ define('ABSPATH', '/tmp/fake-wp/');
+}📝 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.
| define('ABSPATH', '/tmp/fake-wp/'); | |
| if (!defined('ABSPATH')) { | |
| define('ABSPATH', '/tmp/fake-wp/'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/bootstrap.php` at line 3, The tests/bootstrap.php file unconditionally
defines the ABSPATH constant which can trigger "already defined" warnings; wrap
the definition in a guard that checks defined('ABSPATH') before calling define
so ABSPATH is only set when not already present (replace the current
define('ABSPATH', '/tmp/fake-wp/'); with an if (!defined('ABSPATH')) {
define('ABSPATH', '/tmp/fake-wp/'); } pattern to avoid redefinition warnings).
|
This pull request has been marked as stale due to inactivity. It will be closed in 30 days if no further updates are made. |
Add a deps job that installs node and composer dependencies once and caches node_modules and vendor. The lint, test, and build jobs now run after deps and restore from cache with fail-on-cache-miss. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary by CodeRabbit