Add unit tests for query region handling#54
Conversation
WalkthroughAdds a PHPUnit test setup: composer.json gains phpunit/phpunit (^9.6) in require-dev, updates PHP requirement to >=8.3, and adds a "test" script; phpunit.xml and tests/bootstrap.php are added; tests/MultipleQueryRegionTest.php introduces two PHPUnit tests and a WP_HTML_Tag_Processor test helper; .gitignore ignores .phpunit.result.cache. CI updates: .github/workflows/build-release.yaml adds PHP setup, installs dev deps, and runs tests earlier; a new .github/workflows/test.yml workflow runs composer install, validate, and composer test on pull requests to trunk. No exported/public API changes. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
.github/workflows/build-release.yaml (1)
12-12: Update checkout to v4actions/checkout@v3 is deprecated on GitHub-hosted runners. Use v4.
- - uses: actions/checkout@v3 + - uses: actions/checkout@v4
🧹 Nitpick comments (6)
composer.json (1)
62-63: Use the project-local PHPUnit binaryMake the test script explicitly use the project’s vendor binary to avoid falling back to any global phpunit on the runner.
- "test": "phpunit", + "test": "vendor/bin/phpunit --colors=always",tests/bootstrap.php (2)
27-33: Limit attribute injection to first match; current str_replace affects all occurrencesUsing str_replace here will modify every wp-block-query occurrence in the buffer. For robustness (and parity with a single tag mutation), limit to the first match.
- public function set_attribute($name, $value) { - $this->html = str_replace( - '<div class="wp-block-query"', - "<div class=\"wp-block-query\" {$name}=\"{$value}\"", - $this->html - ); - } + public function set_attribute(string $name, string $value): void { + // Inject attribute into the first <div class="wp-block-query"...> only. + $this->html = preg_replace( + '/<div\s+class="wp-block-query"(?=\s|>)/', + '<div class="wp-block-query" ' . $name . '="' . $value . '"', + $this->html, + 1 + ); + }
20-25: Add simple type hints for clarityOptional, but these improve readability and correctness in strict_types mode.
- public function next_tag($args = []) { + public function next_tag(array $args = []): bool { if (isset($args['class_name']) && $args['class_name'] === 'wp-block-query') { return strpos($this->html, 'wp-block-query') !== false; } return false; } @@ - public function get_updated_html() { + public function get_updated_html(): string { return $this->html; }Also applies to: 35-37
tests/MultipleQueryRegionTest.php (3)
10-13: Prefer protected setUp for PHPUnit lifecyclePublic works, but PHPUnit convention is protected setUp(): void.
- public function setUp(): void { + protected function setUp(): void { require_once __DIR__ . '/../src/Plugin.php'; $this->plugin = \WPcomSpecialProjects\Qllm\Plugin::get_instance(); }
6-13: Optional: add a typed property or docblock for $pluginClarifies intent and enables static analysis.
-class MultipleQueryRegionTest extends TestCase { - - private $plugin; +class MultipleQueryRegionTest extends TestCase { + /** @var \WPcomSpecialProjects\Qllm\Plugin */ + private $plugin;
79-88: DRY up repeated region extractionYou repeat the same preg_match 4 times in each test. A small helper keeps tests concise.
+ private function extractRegion(string $html): ?string { + if (preg_match('/data-qllm-query-region="([^"]*)"/', $html, $m)) { + return $m[1]; + } + return null; + } @@ - preg_match('/data-qllm-query-region="([^"]*)"/', $stickyResult, $stickyMatches); - preg_match('/data-qllm-query-region="([^"]*)"/', $normalResult, $normalMatches); - preg_match('/data-qllm-query-region="([^"]*)"/', $normalPage2Result, $normalPage2Matches); - preg_match('/data-qllm-query-region="([^"]*)"/', $normalPage3Result, $normalPage3Matches); - - $stickyRegion = $stickyMatches[1] ?? null; - $normalRegion = $normalMatches[1] ?? null; - $normalPage2Region = $normalPage2Matches[1] ?? null; - $normalPage3Region = $normalPage3Matches[1] ?? null; + $stickyRegion = $this->extractRegion($stickyResult); + $normalRegion = $this->extractRegion($normalResult); + $normalPage2Region = $this->extractRegion($normalPage2Result); + $normalPage3Region = $this->extractRegion($normalPage3Result);Also applies to: 121-130
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/build-release.yaml(1 hunks).github/workflows/test.yml(1 hunks).gitignore(1 hunks)composer.json(2 hunks)phpunit.xml(1 hunks)tests/MultipleQueryRegionTest.php(1 hunks)tests/bootstrap.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/MultipleQueryRegionTest.php (1)
src/Plugin.php (3)
Plugin(19-326)get_instance(63-71)render_query_block(314-325)
🪛 actionlint (1.7.7)
.github/workflows/test.yml
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
phpunit.xml (1)
2-8: Minimal PHPUnit config looks goodBootstrap and testsuite are correctly wired for the added tests.
.gitignore (1)
383-385: Ignore PHPUnit result cacheGood call adding .phpunit.result.cache to keep noise out of VCS.
tests/MultipleQueryRegionTest.php (1)
61-100: Tests align with PR objectiveBoth scenarios validate that data-qllm-query-region is unique across blocks and pages, independent of queryId, matching the intended behavior.
Also applies to: 102-142
| - name: Run tests | ||
| run: composer test | ||
|
|
There was a problem hiding this comment.
Tests run without dev deps or PHP setup
phpunit isn’t installed unless you first run composer install with dev dependencies, and PHP isn’t configured. Add setup-php and a dev install before running tests. Keep the later --no-dev install for packaging.
- name: Validate composer.json and composer.lock
run: composer validate --strict
+ - name: Set up PHP
+ uses: shivammathur/setup-php@v2
+ with:
+ php-version: '8.3' # Align with PHPUnit version
+ coverage: none
+ tools: composer
+
+ - name: Install dependencies (with dev) for tests
+ run: composer install --no-interaction --prefer-dist
+
- name: Run tests
- run: composer test
+ run: composer test📝 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: Run tests | |
| run: composer test | |
| - name: Validate composer.json and composer.lock | |
| run: composer validate --strict | |
| - name: Set up PHP | |
| uses: shivammathur/setup-php@v2 | |
| with: | |
| php-version: '8.3' # Align with PHPUnit version | |
| coverage: none | |
| tools: composer | |
| - name: Install dependencies (with dev) for tests | |
| run: composer install --no-interaction --prefer-dist | |
| - name: Run tests | |
| run: composer test |
🤖 Prompt for AI Agents
In .github/workflows/build-release.yaml around lines 17-19 the workflow runs
tests with "composer test" but misses PHP setup and dev dependencies; modify the
job to use actions/setup-php (set required PHP version), run composer install
(with dev deps) before the test step so phpunit and other dev tools are
available, then after tests continue with the existing packaging step that runs
composer install --no-dev; ensure the setup-php step precedes composer install
and the composer test step.
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Validate composer.json and composer.lock | ||
| run: composer validate --strict | ||
|
|
||
| - name: Run tests | ||
| run: composer test |
There was a problem hiding this comment.
Tests won’t run: missing PHP setup and composer install (also update checkout@v3 to v4)
- actionlint: checkout@v3 is outdated; use v4.
- composer test runs without installing dev dependencies; phpunit won’t be available.
- No PHP toolchain setup; pin a PHP version compatible with your chosen PHPUnit.
Apply this diff to fix the workflow:
name: Test
on:
pull_request:
branches: [ trunk ]
jobs:
test:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4
+ - name: Set up PHP
+ uses: shivammathur/setup-php@v2
+ with:
+ php-version: '8.3' # Adjust if you choose a different PHPUnit version
+ coverage: none
+ tools: composer
+
- name: Validate composer.json and composer.lock
run: composer validate --strict
+ - name: Install dependencies (with dev)
+ run: composer install --no-interaction --prefer-dist
+
- name: Run tests
- run: composer test
+ run: composer test📝 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.
| steps: | |
| - uses: actions/checkout@v3 | |
| - name: Validate composer.json and composer.lock | |
| run: composer validate --strict | |
| - name: Run tests | |
| run: composer test | |
| name: Test | |
| on: | |
| pull_request: | |
| branches: [ trunk ] | |
| jobs: | |
| test: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Set up PHP | |
| uses: shivammathur/setup-php@v2 | |
| with: | |
| php-version: '8.3' # Adjust if you choose a different PHPUnit version | |
| coverage: none | |
| tools: composer | |
| - name: Validate composer.json and composer.lock | |
| run: composer validate --strict | |
| - name: Install dependencies (with dev) | |
| run: composer install --no-interaction --prefer-dist | |
| - name: Run tests | |
| run: composer test |
🧰 Tools
🪛 actionlint (1.7.7)
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/test.yml lines 11-18: the workflow is missing PHP setup,
composer install (dev deps) and uses an outdated checkout action; update
actions/checkout@v3 to v4, add a step to set up PHP via actions/setup-php with a
pinned PHP version compatible with your PHPUnit, ensure composer is available,
add a step to run composer install (including dev dependencies) before running
composer validate/ test (eg. composer install --prefer-dist --no-progress
--no-suggest --no-interaction), then run composer validate --strict and composer
test; optionally add composer cache between runs to speed CI.
| public function setUp(): void { | ||
| require_once __DIR__ . '/../src/Plugin.php'; | ||
| $this->plugin = \WPcomSpecialProjects\Qllm\Plugin::get_instance(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify render_query_block() signature matches test usage
Tests call render_query_block($block_content, $block). The provided snippet of src/Plugin.php shows render_query_block( $block_content ) with a single parameter. If the source still has one parameter, tests will fail with ArgumentCountError.
If it indeed takes a single parameter, either:
- Update tests to call with one argument, or
- Update the plugin method signature to accept (string $block_content, object|array $block) and ignore the second arg if not needed.
Also applies to: 61-78, 111-120
🏁 Script executed:
#!/bin/bash
# Verify the signature of render_query_block
rg -n -A3 -B3 $'function\\s+render_query_block\\s*\\(' src/Plugin.php
# Additionally list all definitions to confirm overloads/hooks (if any)
rg -n $'render_query_block\\s*\\(' srcLength of output: 375
Ensure render_query_block() accepts two parameters to match tests
The render_query_block() method in src/Plugin.php currently only accepts one argument, but your tests invoke it with two ($block_content, $block), causing an ArgumentCountError. To resolve this, update the method signature (and its docblock) to accept the second parameter—optionally with a default—to maintain backward compatibility.
Locations to update:
- src/Plugin.php at line 314
- Optional: tests in tests/MultipleQueryRegionTest.php (if you’d rather adjust tests instead)
Suggested diff for src/Plugin.php:
--- a/src/Plugin.php
+++ b/src/Plugin.php
@@
- /**
- * @return string
- */
- public function render_query_block( $block_content ) {
+ /**
+ * @param string $block_content The block’s raw HTML.
+ * @param stdClass|array|null $block The block attributes/context (optional).
+ * @return string
+ */
+ public function render_query_block( $block_content, $block = null ) {This change allows two arguments (content and block data) without breaking existing one-arg calls.
🤖 Prompt for AI Agents
In src/Plugin.php around line 314, update the render_query_block() method
signature and its docblock so it accepts a second parameter (e.g. $block = null)
to match tests calling render_query_block($block_content, $block); change the
docblock to document the two parameters and that $block is optional, and ensure
any internal references use the new $block variable safely (check for null) so
one-arg calls remain compatible.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/bootstrap.php (1)
20-25: Simplified implementation may miss edge cases.The
next_tagmethod uses a basic string search which may produce false positives if the class name appears in other contexts (e.g., within attribute values or text content).For more robust testing, consider using a regex pattern to match actual HTML class attributes:
public function next_tag($args = []) { if (isset($args['class_name'])) { - return strpos($this->html, $args['class_name']) !== false; + $pattern = '/class=["\'][^"\']*\b' . preg_quote($args['class_name'], '/') . '\b[^"\']*["\']/'; + return preg_match($pattern, $this->html); } return false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/MultipleQueryRegionTest.php(1 hunks)tests/bootstrap.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/MultipleQueryRegionTest.php
🔇 Additional comments (1)
tests/bootstrap.php (1)
4-10: LGTM! Proper constants definition for test environment.The conditional definition of both constants follows best practices and provides appropriate default values for testing.
| public function set_attribute($name, $value) { | ||
| if (strpos($this->html, 'wp-block-post-template') !== false) { | ||
| $this->html = str_replace( | ||
| '<div class="wp-block-post-template"', | ||
| "<div class=\"wp-block-post-template\" {$name}=\"{$value}\"", | ||
| $this->html | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add HTML escaping to prevent malformed HTML.
The set_attribute method lacks HTML escaping for attribute values, which could generate malformed HTML if values contain special characters like quotes.
Apply this diff to add proper HTML escaping:
public function set_attribute($name, $value) {
if (strpos($this->html, 'wp-block-post-template') !== false) {
+ $escaped_name = htmlspecialchars($name, ENT_QUOTES);
+ $escaped_value = htmlspecialchars($value, ENT_QUOTES);
$this->html = str_replace(
'<div class="wp-block-post-template"',
- "<div class=\"wp-block-post-template\" {$name}=\"{$value}\"",
+ "<div class=\"wp-block-post-template\" {$escaped_name}=\"{$escaped_value}\"",
$this->html
);
}
}📝 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.
| public function set_attribute($name, $value) { | |
| if (strpos($this->html, 'wp-block-post-template') !== false) { | |
| $this->html = str_replace( | |
| '<div class="wp-block-post-template"', | |
| "<div class=\"wp-block-post-template\" {$name}=\"{$value}\"", | |
| $this->html | |
| ); | |
| } | |
| } | |
| public function set_attribute($name, $value) { | |
| if (strpos($this->html, 'wp-block-post-template') !== false) { | |
| $escaped_name = htmlspecialchars($name, ENT_QUOTES); | |
| $escaped_value = htmlspecialchars($value, ENT_QUOTES); | |
| $this->html = str_replace( | |
| '<div class="wp-block-post-template"', | |
| "<div class=\"wp-block-post-template\" {$escaped_name}=\"{$escaped_value}\"", | |
| $this->html | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In tests/bootstrap.php around lines 27 to 35, the set_attribute method inserts
raw attribute values into HTML which can break markup when values contain quotes
or other special chars; fix it by HTML-escaping the incoming value and
validating the attribute name before insertion: run the attribute value through
htmlspecialchars($value, ENT_QUOTES, 'UTF-8') (or equivalent) and
sanitize/validate $name to allow only safe name characters (e.g. [A-Za-z0-9_-])
and if invalid, skip the change; then use the escaped name/value when performing
the replacement so the produced HTML remains well-formed.
jshakes
force-pushed
the
js-multiple-region-tests-and-fix
branch
from
79ae24c to
23dfebc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
14-19: Fix: Use the correct PHP setup action (shivammathur/setup-php), not actions/setup-phpThe action
actions/setup-php@v2does not exist and will fail the job. Useshivammathur/setup-php@v2which is the de facto standard for PHP on GitHub Actions.Apply this diff:
- - name: Setup PHP - uses: actions/setup-php@v2 - with: - php-version: '8.0' - tools: composer + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.0' + tools: composer
🧹 Nitpick comments (5)
.github/workflows/test.yml (3)
20-21: Fail fast: run composer validate before installing dependenciesValidation should precede installation to catch manifest issues early and save CI time.
- - name: Install dependencies - run: composer install --prefer-dist --no-progress --no-interaction - - - name: Validate composer.json and composer.lock - run: composer validate --strict + - name: Validate composer.json and composer.lock + run: composer validate --strict + + - name: Install dependencies + run: composer install --prefer-dist --no-progress --no-interactionAlso applies to: 23-24
20-21: Speed up installs with Composer cachingLeverage a caching step to significantly reduce CI time on repeated runs.
Replace the install step with ramsey/composer-install (it handles caching):
- - name: Install dependencies - run: composer install --prefer-dist --no-progress --no-interaction + - name: Install dependencies + uses: ramsey/composer-install@v3 + with: + composer-options: --prefer-dist --no-progress --no-interaction
17-17: Optional: Test across supported PHP versions with a matrixConsider a version matrix (e.g., 8.0–8.3) to catch compatibility issues early.
Example:
jobs: - test: + test: + strategy: + matrix: + php: ['8.0', '8.1', '8.2', '8.3'] runs-on: ubuntu-latest @@ - - name: Setup PHP + - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: '8.0' + php-version: ${{ matrix.php }} tools: composer.github/workflows/build-release.yaml (2)
19-21: Reorder: validate before installing dev dependenciesValidate manifests first to fail fast; install dev deps after validation and before tests.
- - name: Install dev dependencies - run: composer install - - name: Validate composer.json and composer.lock run: composer validate --strict + + - name: Install dev dependencies + run: composer install --no-interaction --prefer-distAlso applies to: 22-23
19-21: Speed up Composer with cachingUse ramsey/composer-install for cached installs; helpful especially in release runs with test steps.
- - name: Install dev dependencies - run: composer install --no-interaction --prefer-dist + - name: Install dev dependencies + uses: ramsey/composer-install@v3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-release.yaml(1 hunks).github/workflows/test.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/build-release.yaml (1)
25-26: Tests step ordering looks good once PHP setup and dev install are fixedWith PHP properly set up and dev deps installed, running
composer testhere is correct. Keep the later production install for packaging.
| @@ -11,9 +11,20 @@ jobs: | |||
| steps: | |||
| - uses: actions/checkout@v3 | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Upgrade checkout action to v4
actions/checkout@v3 is deprecated on modern runners and will trigger actionlint warnings. Use v4.
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4📝 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.
| - uses: actions/checkout@v3 | |
| - uses: actions/checkout@v4 |
🧰 Tools
🪛 actionlint (1.7.7)
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/build-release.yaml around line 12, the workflow uses
actions/checkout@v3 which is deprecated; update the step to use
actions/checkout@v4 instead. Edit that line to replace the version tag from v3
to v4 so the workflow uses the latest checkout action and resolves actionlint
warnings.
| - name: Setup PHP | ||
| uses: actions/setup-php@v2 | ||
| with: | ||
| php-version: '8.0' | ||
|
|
There was a problem hiding this comment.
Fix: Use the correct PHP setup action and ensure Composer is available
Switch to shivammathur/setup-php@v2 and add tools: composer to guarantee Composer availability on the runner.
- - name: Setup PHP
- uses: actions/setup-php@v2
- with:
- php-version: '8.0'
+ - name: Setup PHP
+ uses: shivammathur/setup-php@v2
+ with:
+ php-version: '8.0'
+ tools: composer📝 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: Setup PHP | |
| uses: actions/setup-php@v2 | |
| with: | |
| php-version: '8.0' | |
| - name: Setup PHP | |
| uses: shivammathur/setup-php@v2 | |
| with: | |
| php-version: '8.0' | |
| tools: composer |
🤖 Prompt for AI Agents
.github/workflows/build-release.yaml around lines 14 to 18: the workflow uses
actions/setup-php@v2 which doesn't guarantee Composer availability; replace it
with shivammathur/setup-php@v2 and specify tools: composer in the step so the
runner has Composer installed and configured; update the step's uses value to
shivammathur/setup-php@v2 and add the tools: composer entry under with.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/build-release.yaml (1)
12-12: Upgrade checkout action to v4actions/checkout@v3 is deprecated on modern runners. Use v4.
- - uses: actions/checkout@v3 + - uses: actions/checkout@v4
🧹 Nitpick comments (3)
composer.json (1)
62-63: Nit: Prefer explicit path for PHPUnit binaryComposer scripts add vendor/bin to PATH, but making the script explicit improves clarity and portability across environments.
- "test": "phpunit", + "test": "vendor/bin/phpunit",.github/workflows/build-release.yaml (2)
14-18: Add Composer tool and cache to PHP setupEnsure Composer is present and speed up installs with caching.
- name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: '8.3' + tools: composer + coverage: none + ini-values: memory_limit=-1 + extensions: mbstring, json + # Optional: built-in Composer cache + # https://github.com/shivammathur/setup-php#composer-cache + # Having this line enables caching automatically: + # (no diff needed if you prefer a separate cache step)
19-27: Reorder validation before install; harden Composer install flagsRun composer validate before installing dependencies, and use robust flags for non-interactive CI runs.
- - name: Install dev dependencies - run: composer install - - name: Validate composer.json and composer.lock run: composer validate --strict - - name: Run tests - run: composer test + - name: Install dev dependencies + run: composer install --no-interaction --prefer-dist --no-progress + + - name: Run tests + run: composer test
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/build-release.yaml(1 hunks).github/workflows/test.yml(1 hunks)composer.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
🔇 Additional comments (1)
composer.json (1)
40-41: LGTM: PHPUnit dev dependencyPinning phpunit/phpunit to ^9.6 is a sensible choice and aligns with broad PHP 8.x compatibility. No issues.
| "php": ">=8.3", | ||
| "ext-json": "*" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm intent to raise runtime requirement to PHP 8.3
You’ve bumped the project’s runtime requirement to PHP >=8.3. Given that:
- PHPUnit ^9.6 does not require PHP 8.3 (it supports older 8.x),
- Dev dependency johnpbloch/wordpress-core is pinned to 6.2.*, which historically did not officially support PHP 8.3,
please verify this is intentional for production installs (not just CI). If broader runtime support is desired, consider reverting to your previous boundary (e.g., >=8.0) and keep PHPUnit at ^9.6.
If you decide to revert:
- "php": ">=8.3",
+ "php": ">=8.0",To validate WordPress compatibility with PHP 8.3, please run this web query:
🌐 Web query:
What is the first WordPress version to officially support PHP 8.3?
💡 Result:
WordPress 6.8 — it was the first WordPress release documented as fully supporting PHP 8.3. (make.wordpress.org)
For context: WordPress 6.8 was released April 15, 2025. (make.wordpress.org)
The Core team published and implemented the proposal to remove the “beta support” label for PHP 8.3 in July 2025. (make.wordpress.org)
Citations:
- 1: https://make.wordpress.org/core/2025/04/09/php-8-support-clarification/?utm_source=chatgpt.com
- 2: https://make.wordpress.org/hosting/2025/04/16/wordpress-6-8-server-compatibility/?utm_source=chatgpt.com
- 3: https://make.wordpress.org/core/2025/07/07/proposal-remove-the-beta-support-label-from-php-8-3-for-wordpress-6-8/
Confirm PHP 8.3 bump or update WordPress Core dependency
You’ve raised the project’s PHP requirement to >=8.3 in composer.json, but you’re still pinning johnpbloch/wordpress-core to 6.2.*, and WordPress 6.2 does not officially support PHP 8.3. The first WP release with full PHP 8.3 support is WordPress 6.8 (April 2025) [1][2][3].
Please choose one of the following:
- If you intend to require PHP 8.3 for production installs, also update the WordPress Core dev dependency to at least 6.8:
"require-dev": { - "johnpbloch/wordpress-core": "6.2.*" + "johnpbloch/wordpress-core": "^6.8" } - If you need to maintain compatibility with WordPress 6.2, revert the PHP requirement to your previous boundary (e.g.,
>=8.0):- "php": ">=8.3", + "php": ">=8.0",
Let me know which direction you’d like to take.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In composer.json around lines 30-31, you bumped the PHP requirement to ">=8.3"
but still pin "johnpbloch/wordpress-core" to 6.2.*, which is incompatible with
PHP 8.3; either update the WordPress core dependency to at least 6.8 (change the
package constraint to >=6.8 and run composer update, then run tests) if you
intend to require PHP 8.3, or revert the PHP requirement back to the previous
safe boundary (e.g., ">=8.0") if you must remain compatible with WP 6.2; apply
one of these changes in composer.json and run composer update and your test
suite to verify.
tommusrhodus
left a comment
tommusrhodus
left a comment
There was a problem hiding this comment.
Thank you for writing these tests :)
2 participants
Adds unit tests for #53 - specifically ensuring that a Query Loop block's
queryIDhas no bearing on the value of thedata-qllm-query-regionattributes, which should be unique across multiple pages and even when a page contains two Query Loop blocks with identicalqueryIDs.Adds the unit test runner (available via
composer test) to the Github Action workflows for PRs.Summary by CodeRabbit