Skip to content

Add missing tests#77

Merged
gin0115 merged 1 commit into
trunkfrom
feature/add-tests-for-calendar-start-date
Jun 10, 2026
Merged

Add missing tests#77
gin0115 merged 1 commit into
trunkfrom
feature/add-tests-for-calendar-start-date

Conversation

@gin0115

@gin0115 gin0115 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Changes proposed in this Pull Request

This pull request introduces comprehensive automated testing for the calendar's month grid alignment with the WordPress start_of_week setting. It adds both end-to-end (Playwright) and PHP unit tests to ensure that the calendar grid correctly starts and ends on the expected days of the week, contains all days of the month, and handles edge cases across all possible start_of_week values. Additionally, it sets up a GitHub Actions workflow to run PHPUnit tests on pull requests.

Testing infrastructure and workflow:

  • Adds a GitHub Actions workflow (.github/workflows/phpunit.yml) to automatically run PHPUnit tests on pull requests targeting the trunk and develop branches, ensuring CI coverage for PHP code.

End-to-end (E2E) testing:

  • Introduces a Playwright E2E test (tests/e2e/specs/frontend/calendar-start-of-week.spec.js) that seeds the calendar for different start_of_week values and verifies:
    • Header columns are rotated correctly.
    • Each week has exactly 7 cells.
    • The grid aligns with the correct start and end weekdays.
    • The first in-month day lands in the expected column.
    • The grid covers a whole number of weeks.
  • Adds a PHP seeder script (tests/e2e/fixtures/seed-calendar.php) to reset the calendar state, set the start_of_week option, seed a test event, and create a test page for E2E navigation.

PHP unit testing:

  • Adds a PHPUnit test class (tests/phpunit/Calendar/CalendarGridTest.php) that:
    • Tests grid alignment for all start_of_week values and various months.
    • Ensures every day of the month appears exactly once.
    • Verifies the first of the month lands in the correct column.
    • Covers the Sunday-start edge case where the first day is Sunday.

Testing instructions

Mentions #

Summary by CodeRabbit

  • Tests

    • Added PHP unit tests to verify calendar grid alignment with WordPress start-of-week settings.
    • Added end-to-end tests to validate calendar rendering across different day-start configurations.
  • Chores

    • Configured automated testing workflow for continuous integration.
    • Updated project configuration to exclude development utilities.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

This PR adds comprehensive test coverage for calendar grid alignment with WordPress' start_of_week option. It configures a GitHub Actions workflow to run PHP unit tests on pull requests, creates an idempotent e2e fixture that seeds test data with configurable start_of_week values, implements Playwright end-to-end tests that verify calendar header rotation and day cell placement, and adds PHPUnit regression tests validating grid structure across all weekday start configurations (0–6, Sunday through Saturday).

Possibly related PRs

  • a8cteam51/simple-events#76: Directly related; this PR adds PHPUnit and e2e test coverage for the calendar boundary logic that was modified in that PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Add missing tests' is vague and generic, using a non-descriptive term that doesn't convey what tests were added or their purpose. Improve the title to be more specific, such as 'Add tests for calendar start-of-week alignment' or 'Add PHPUnit and E2E tests for calendar grid alignment with start_of_week setting'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
tests/e2e/specs/frontend/calendar-start-of-week.spec.js (1)

2-3: ⚡ Quick win

Use execFileSync with an argument array instead of shell interpolation.

This avoids shell parsing and makes the seeder invocation more robust while preserving behavior.

Proposed patch
-const { execSync } = require( 'child_process' );
+const { execFileSync } = require( 'child_process' );
 const path = require( 'path' );
@@
-	const out = execSync(
-		`npx wp-env run cli --env-cwd='wp-content/plugins/simple-events' -- wp eval-file tests/e2e/fixtures/seed-calendar.php ${ startOfWeek }`,
-		{ encoding: 'utf8' }
-	);
+	const out = execFileSync(
+		'npx',
+		[
+			'wp-env',
+			'run',
+			'cli',
+			'--env-cwd=wp-content/plugins/simple-events',
+			'--',
+			'wp',
+			'eval-file',
+			'tests/e2e/fixtures/seed-calendar.php',
+			String( startOfWeek ),
+		],
+		{ encoding: 'utf8' }
+	);

Also applies to: 29-32

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/e2e/specs/frontend/calendar-start-of-week.spec.js` around lines 2 - 3,
Replace uses of execSync with execFileSync and pass the command and arguments as
an array to avoid shell interpolation: locate where execSync is used to invoke
the seeder (references to execSync in this test file) and change the call to
require('child_process').execFileSync, passing the executable as the first arg
and the argv array (e.g., ['arg1','arg2', ...]) as the second parameter;
preserve existing env and options by forwarding the same options object to
execFileSync. Ensure all occurrences in this file (including the similar block
around the previous invocation) are updated.

Source: Linters/SAST tools

tests/phpunit/Calendar/CalendarGridTest.php (1)

86-86: 💤 Low value

Consider using array index access instead of end() for clarity.

The end() function advances the array's internal pointer to retrieve the last element. While safe in this context, using $days[ count( $days ) - 1 ] is more explicit and avoids pointer side effects.

♻️ Proposed refactor
-			$last_weekday = (int) end( $days )['date']->format( 'w' );
+			$last_weekday = (int) $days[ count( $days ) - 1 ]['date']->format( 'w' );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/phpunit/Calendar/CalendarGridTest.php` at line 86, The test uses
end($days) to get the last element when computing $last_weekday which relies on
advancing the array pointer; replace that with explicit index access like
$days[count($days) - 1] to be clearer and avoid pointer side-effects (reference
the $last_weekday variable and the $days array), and while updating ensure you
guard against empty $days (e.g., check count($days) > 0) before accessing and
keep the existing cast to (int) and ->format('w') usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/phpunit.yml:
- Around line 13-23: The workflow uses floating tags for actions
(actions/checkout@v4, shivammathur/setup-php@v2, actions/setup-node@v4); update
each "uses" invocation to a pinned full commit SHA for actions/checkout,
shivammathur/setup-php, and actions/setup-node to comply with the repository
policy and improve supply-chain integrity—locate the three uses lines in the
phpunit.yml and replace the tag versions with their respective commit SHAs from
the official repos.
- Line 13: The workflow step currently uses actions/checkout@v4 without
disabling credential persistence; update the checkout step (actions/checkout@v4)
to include persist-credentials: false so the job token is not stored in local
git config for later steps, e.g., add the persist-credentials: false input under
the checkout step.

---

Nitpick comments:
In `@tests/e2e/specs/frontend/calendar-start-of-week.spec.js`:
- Around line 2-3: Replace uses of execSync with execFileSync and pass the
command and arguments as an array to avoid shell interpolation: locate where
execSync is used to invoke the seeder (references to execSync in this test file)
and change the call to require('child_process').execFileSync, passing the
executable as the first arg and the argv array (e.g., ['arg1','arg2', ...]) as
the second parameter; preserve existing env and options by forwarding the same
options object to execFileSync. Ensure all occurrences in this file (including
the similar block around the previous invocation) are updated.

In `@tests/phpunit/Calendar/CalendarGridTest.php`:
- Line 86: The test uses end($days) to get the last element when computing
$last_weekday which relies on advancing the array pointer; replace that with
explicit index access like $days[count($days) - 1] to be clearer and avoid
pointer side-effects (reference the $last_weekday variable and the $days array),
and while updating ensure you guard against empty $days (e.g., check
count($days) > 0) before accessing and keep the existing cast to (int) and
->format('w') usage.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e373b7f6-efe3-4320-ba7b-2b984c4f4b65

📥 Commits

Reviewing files that changed from the base of the PR and between 5572c9b and 7d6a930.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • .github/workflows/phpunit.yml
  • .gitignore
  • tests/e2e/fixtures/seed-calendar.php
  • tests/e2e/specs/frontend/calendar-start-of-week.spec.js
  • tests/phpunit/Calendar/CalendarGridTest.php

timeout-minutes: 30

steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Disable persisted checkout credentials.

actions/checkout should set persist-credentials: false so the job token is not left in local git config for subsequent steps.

Suggested hardening
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
📝 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.

Suggested change
- uses: actions/checkout@v4
- uses: actions/checkout@v4
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 13-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/phpunit.yml at line 13, The workflow step currently uses
actions/checkout@v4 without disabling credential persistence; update the
checkout step (actions/checkout@v4) to include persist-credentials: false so the
job token is not stored in local git config for later steps, e.g., add the
persist-credentials: false input under the checkout step.

Source: Linters/SAST tools

Comment on lines +13 to +23
- uses: actions/checkout@v4

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.3'
tools: composer

- name: Setup Node.js
uses: actions/setup-node@v4
with:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin all GitHub Actions to full commit SHAs.

Using floating tags (@v4, @v2) violates the repository’s stated policy and weakens supply-chain integrity in CI.

Suggested hardening
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<full_commit_sha>

-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@<full_commit_sha>

-        uses: actions/setup-node@v4
+        uses: actions/setup-node@<full_commit_sha>
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 13-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 22-22: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/phpunit.yml around lines 13 - 23, The workflow uses
floating tags for actions (actions/checkout@v4, shivammathur/setup-php@v2,
actions/setup-node@v4); update each "uses" invocation to a pinned full commit
SHA for actions/checkout, shivammathur/setup-php, and actions/setup-node to
comply with the repository policy and improve supply-chain integrity—locate the
three uses lines in the phpunit.yml and replace the tag versions with their
respective commit SHAs from the official repos.

Source: Linters/SAST tools

@gin0115 gin0115 merged commit d87e9f7 into trunk Jun 10, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant