Update rector config to actually fix issues in tests#2978
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe PR refactors test assertions across the codebase to use PHPUnit-specific assertion methods (such as Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 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. ✨ 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 |
 |
|
Overall Grade Focus Area: Security |
Security  Reliability  Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Feb 24, 2026 7:39p.m. | Review ↗ |
| JavaScript |  |
Feb 24, 2026 7:39p.m. | Review ↗ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/phpunit/fields/test_FrmFieldValidate.php (1)
183-234:⚠️ Potential issue | 🟡 MinorFix typo on line 198: "Email email" should be "Empty email".
The assertion on line 198 contains a typo in the failure message:
'Email email passed required validation'should be'Empty email passed required validation'to accurately describe the test case checking that an empty value fails validation for a required email field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/fields/test_FrmFieldValidate.php` around lines 183 - 234, In test_email_value(), fix the failure message for the required-email assertion by replacing the typo "Email email passed required validation" with "Empty email passed required validation" so the assertArrayHasKey call for $this->check_single_value( array( $field->id => '' ) ) accurately describes the test case in the method test_email_value.tests/phpunit/misc/test_FrmAppHelper.php (1)
572-576:⚠️ Potential issue | 🟠 MajorFix PHPCS spacing in assertLessThanOrEqual.
Line 576 fails PHPCS because the function call is missing spaces after the opening parenthesis and before the closing parenthesis.
🔧 Suggested fix
- $this->assertLessThanOrEqual(70, strlen( $unique_key )); + $this->assertLessThanOrEqual( 70, strlen( $unique_key ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/misc/test_FrmAppHelper.php` around lines 572 - 576, The PHPCS spacing error is caused by the assertLessThanOrEqual call missing spaces after the opening parenthesis and before the closing parenthesis; update the test assertion line that calls $this->assertLessThanOrEqual(70, strlen( $unique_key )); (which follows the FrmAppHelper::get_unique_key call) so it uses proper spacing inside the parentheses (space after '(' and before ')') to satisfy PHPCS.
🧹 Nitpick comments (2)
tests/phpunit/forms/test_FrmOnSubmitAction.php (1)
46-47:assertNotEmptyis redundant; considerassertArrayHasKeyinstead.
assertNotEmpty($post_content['success_url'])adds no coverage beyond theassertSameon the next line — any empty or wrong value is already caught there. A more useful guard would beassertArrayHasKey, which both avoids a potential undefined-index notice and produces a clearer failure message for the distinct case where the key itself is absent.♻️ Proposed refactor
- $this->assertNotEmpty( $post_content['success_url'] ); + $this->assertArrayHasKey( 'success_url', $post_content ); $this->assertSame( 'https://example.com/?param=[' . $field_id . ' sanitize_url=1]', $post_content['success_url'] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/forms/test_FrmOnSubmitAction.php` around lines 46 - 47, The test uses assertNotEmpty($post_content['success_url']) which is redundant and risky if the key is missing; replace that assertion with assertArrayHasKey('success_url', $post_content) to explicitly verify the array contains the key before the subsequent assertSame check (refer to $post_content, 'success_url', assertNotEmpty, assertArrayHasKey, and assertSame in test_FrmOnSubmitAction.php).tests/phpunit/forms/test_FrmForm.php (1)
63-66: Tighten the “deleted form” assertion.
assertNotInstanceOf(\stdClass::class, …)passes for any non‑stdClass value (arrays, WP_Error, etc.), which can allow false positives ifFrmForm::getOneever returns another non‑empty type. If the contract is falsy on missing, assert that explicitly (or useassertEmpty) to keep the deletion check strict.Possible tightening (adjust to the actual contract)
- $this->assertNotInstanceOf( \stdClass::class, $form_exists, 'Failed to delete form ' . $form->form_key ); + $this->assertEmpty( $form_exists, 'Failed to delete form ' . $form->form_key );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/forms/test_FrmForm.php` around lines 63 - 66, The test currently uses assertNotInstanceOf(\stdClass::class, $form_exists) which is too loose; change the assertion to explicitly check the expected "missing" contract from FrmForm::getOne after calling FrmForm::destroy($form->id). Locate the destroy/getOne sequence (FrmForm::destroy and FrmForm::getOne) and replace the assertNotInstanceOf with a strict assertion matching the contract — for example use $this->assertEmpty($form_exists, ...) or $this->assertFalse($form_exists, ...) (or assertNull if getOne returns null) to ensure the deleted form is explicitly falsy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/phpunit/emails/test_FrmEmailSummaryHelper.php`:
- Around line 16-18: The failing PHPCS spacing is due to missing spaces inside
function call parentheses for the assertion calls (e.g.,
assertArrayHasKey('last_monthly', $options)), so update each Rector-generated
call in test_FrmEmailSummaryHelper.php (and similar occurrences in
test_FrmAddon.php and test_FrmField.php) to include a space after the opening
parenthesis and before the closing parenthesis for the function names like
assertArrayHasKey, assertTrue, etc.; make the changes consistently across all
four lines shown and any other occurrences, and then run PHPCS to confirm the
PEAR.Functions.FunctionCallSignature rule is satisfied.
In `@tests/phpunit/entries/test_FrmEntryValidate.php`:
- Around line 276-279: PHPCS flagged missing spaces inside the PHPUnit
assertions; update the calls to assertArrayNotHasKey and assertArrayHasKey so
they have a single space after the opening parenthesis and before the closing
parenthesis (e.g. around the arguments referencing $fields[$key]->id and
$fields['radio_5']->id and the $values['item_meta'] array) to satisfy spacing
rules; modify the calls in the test_FrmEntryValidate.php block where
assertArrayNotHasKey($fields[ $key ]->id, $values['item_meta']) and
assertArrayHasKey($fields['radio_5']->id, $values['item_meta']) appear so
spacing conforms to PHPCS.
In `@tests/phpunit/fields/test_FrmFieldGridHelper.php.php`:
- Around line 111-112: Fix PHPCS spacing for the assertInstanceOf calls by
adding a space after the opening parenthesis and before the closing parenthesis
in the assertion invocation(s); update the calls to assertInstanceOf (and any
similar assertions like the one on lines referenced) so they use the form
assertInstanceOf( <Type>::class, $this->section_helper ); ensuring you adjust
both occurrences (the current assertInstanceOf and the similar assertion at the
other mentioned location).
In `@tests/phpunit/fields/test_FrmFieldValidate.php`:
- Line 198: Fix the typo in the assertion message for the validation test:
update the message passed to $this->assertArrayHasKey that currently reads
'Email email passed required validation ...' to instead read 'Empty email passed
required validation ...', keeping the rest of the message (including
concatenation with print_r($errors, 1)) and references to 'field' . $field->id
and $errors unchanged.
In `@tests/phpunit/misc/test_FrmAddon.php`:
- Line 32: PHPCS is flagging spacing on the test assertion; open the call to
assertArrayHasKey in the test_FrmAddon.php test and fix the spacing so it
matches coding standards (no extra spaces after the opening parenthesis and
exactly one space after the comma between 'signature' and $plugins) for the
method call $this->assertArrayHasKey('signature', $plugins); then run
phpcbf/phpcs to auto-fix or confirm the change.
- Around line 13-14: The test constructs a mock for FrmTestAddon using
getMockBuilder but includes a no-op call to setMethods() which is
deprecated/removed; remove the ->setMethods() call from the chain so the mock is
built as $this->getMockBuilder('FrmTestAddon')->getMock() (i.e., locate the
getMockBuilder('FrmTestAddon') call in tests/phpunit/misc/test_FrmAddon.php and
delete the setMethods() invocation).
In `@tests/phpunit/misc/test_FrmAntiSpam.php`:
- Line 22: Update the PHPCS spacing in the assertion calls so there is a space
after the opening parenthesis and before the closing parenthesis of the
assertion method; e.g. change the assertGreaterThanOrEqual call to use
$this->assertGreaterThanOrEqual( 32, strlen( $token_string ) ); and make the
same spacing adjustment for the other assertion occurrences on the same file
(the other two assertion calls referenced on lines 31 and 40).
In `@tests/phpunit/styles/test_FrmStylesHelper.php`:
- Around line 12-18: The PHPCS failure is caused by missing spaces inside the
assertArrayHasKey call; update the assertion to include a space after the
opening parenthesis and before the closing parenthesis so it reads
$this->assertArrayHasKey( 'baseurl', $base ); while leaving the surrounding test
logic (the FrmStylesHelper::get_upload_base() calls and subsequent assertions)
unchanged.
---
Outside diff comments:
In `@tests/phpunit/fields/test_FrmFieldValidate.php`:
- Around line 183-234: In test_email_value(), fix the failure message for the
required-email assertion by replacing the typo "Email email passed required
validation" with "Empty email passed required validation" so the
assertArrayHasKey call for $this->check_single_value( array( $field->id => '' )
) accurately describes the test case in the method test_email_value.
In `@tests/phpunit/misc/test_FrmAppHelper.php`:
- Around line 572-576: The PHPCS spacing error is caused by the
assertLessThanOrEqual call missing spaces after the opening parenthesis and
before the closing parenthesis; update the test assertion line that calls
$this->assertLessThanOrEqual(70, strlen( $unique_key )); (which follows the
FrmAppHelper::get_unique_key call) so it uses proper spacing inside the
parentheses (space after '(' and before ')') to satisfy PHPCS.
---
Nitpick comments:
In `@tests/phpunit/forms/test_FrmForm.php`:
- Around line 63-66: The test currently uses
assertNotInstanceOf(\stdClass::class, $form_exists) which is too loose; change
the assertion to explicitly check the expected "missing" contract from
FrmForm::getOne after calling FrmForm::destroy($form->id). Locate the
destroy/getOne sequence (FrmForm::destroy and FrmForm::getOne) and replace the
assertNotInstanceOf with a strict assertion matching the contract — for example
use $this->assertEmpty($form_exists, ...) or $this->assertFalse($form_exists,
...) (or assertNull if getOne returns null) to ensure the deleted form is
explicitly falsy.
In `@tests/phpunit/forms/test_FrmOnSubmitAction.php`:
- Around line 46-47: The test uses assertNotEmpty($post_content['success_url'])
which is redundant and risky if the key is missing; replace that assertion with
assertArrayHasKey('success_url', $post_content) to explicitly verify the array
contains the key before the subsequent assertSame check (refer to $post_content,
'success_url', assertNotEmpty, assertArrayHasKey, and assertSame in
test_FrmOnSubmitAction.php).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
classes/models/FrmAntiSpam.phprector.phpstubs.phptests/phpunit/applications/test_FrmApplicationsController.phptests/phpunit/database/test_FrmMigrate.phptests/phpunit/emails/test_FrmEmail.phptests/phpunit/emails/test_FrmEmailSummaryHelper.phptests/phpunit/entries/test_FrmEntry.phptests/phpunit/entries/test_FrmEntryValidate.phptests/phpunit/fields/test_FrmField.phptests/phpunit/fields/test_FrmFieldFormHtml.phptests/phpunit/fields/test_FrmFieldGridHelper.php.phptests/phpunit/fields/test_FrmFieldName.phptests/phpunit/fields/test_FrmFieldValidate.phptests/phpunit/form-templates/test_FrmFormTemplatesController.phptests/phpunit/forms/test_FrmForm.phptests/phpunit/forms/test_FrmFormsController.phptests/phpunit/forms/test_FrmOnSubmitAction.phptests/phpunit/misc/test_FrmAddon.phptests/phpunit/misc/test_FrmAntiSpam.phptests/phpunit/misc/test_FrmAppController.phptests/phpunit/misc/test_FrmAppHelper.phptests/phpunit/misc/test_FrmCSVExportHelper.phptests/phpunit/misc/test_FrmHooksController.phptests/phpunit/misc/test_FrmMisc.phptests/phpunit/misc/test_FrmSimpleBlocksController.phptests/phpunit/misc/test_FrmSpamCheckDenylist.phptests/phpunit/styles/test_FrmStyleApi.phptests/phpunit/styles/test_FrmStylesController.phptests/phpunit/styles/test_FrmStylesHelper.phptests/phpunit/xml/test_FrmXMLHelper.php
|
|
||
| $errors = $this->check_single_value( array( $field->id => '' ) ); | ||
| $this->assertTrue( isset( $errors[ 'field' . $field->id ] ), 'Email email passed required validation ' . print_r( $errors, 1 ) ); | ||
| $this->assertArrayHasKey( 'field' . $field->id, $errors, 'Email email passed required validation ' . print_r( $errors, 1 ) ); |
Contributor
There was a problem hiding this comment.
Typo in error message: "Email email" → "Empty email".
- $this->assertArrayHasKey( 'field' . $field->id, $errors, 'Email email passed required validation ' . print_r( $errors, 1 ) );
+ $this->assertArrayHasKey( 'field' . $field->id, $errors, 'Empty email passed required validation ' . print_r( $errors, 1 ) );📝 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
| $this->assertArrayHasKey( 'field' . $field->id, $errors, 'Email email passed required validation ' . print_r( $errors, 1 ) ); | |
| $this->assertArrayHasKey( 'field' . $field->id, $errors, 'Empty email passed required validation ' . print_r( $errors, 1 ) ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/phpunit/fields/test_FrmFieldValidate.php` at line 198, Fix the typo in
the assertion message for the validation test: update the message passed to
$this->assertArrayHasKey that currently reads 'Email email passed required
validation ...' to instead read 'Empty email passed required validation ...',
keeping the rest of the message (including concatenation with print_r($errors,
1)) and references to 'field' . $field->id and $errors unchanged.
Comment on lines
+13
to
14
| $this->addon = $this->getMockBuilder( 'FrmTestAddon' )->setMethods() | ||
| ->getMock(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the required/installed PHPUnit version
rg -i 'phpunit' composer.json -A 2 -B 2Repository: Strategy11/formidable-forms
Length of output: 234
🌐 Web query:
PHPUnit MockBuilder setMethods deprecated removed version timeline
💡 Result:
PHPUnit MockBuilder::setMethods() deprecation/removal timeline
- Introduced replacements in PHPUnit 8 (2019):
onlyMethods()(mock existing methods) andaddMethods()(mock non-existing methods) were added as the intended split ofsetMethods()responsibilities (see PR#3687referenced by multiple downstream projects). [2] - Deprecated in PHPUnit 8.3.0:
MockBuilder::setMethods()was deprecated (but it notably did not always emit a deprecation warning at runtime, which caused confusion). [1] - Removed in PHPUnit 10.0:
MockBuilder::setMethods()no longer exists; you must migrate toonlyMethods()and/oraddMethods()(or drop the call if unnecessary). [1][2]
Migration mapping
setMethods(['a','b'])→onlyMethods(['a','b'])(when methods exist) [3]- If you previously used
setMethods()to “mock” methods that don’t exist on the class, useaddMethods([...])instead. [3]
Sources: Moodle upgrade notes [1], Drupal issue referencing PR #3687 and removal in PHPUnit 10 [2], PHPUnit manual (onlyMethods() / addMethods()) [3].
🏁 Script executed:
#!/bin/bash
# Get the full composer.json to find PHPUnit version
cat composer.json | jq '.require, .require-dev' 2>/dev/null || head -100 composer.jsonRepository: Strategy11/formidable-forms
Length of output: 1408
🏁 Script executed:
#!/bin/bash
# Also check composer.lock for installed PHPUnit version
rg '"name": "phpunit' composer.lock -A 2 | head -20Repository: Strategy11/formidable-forms
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Find phpunit configuration
fd -name 'phpunit.xml*'Repository: Strategy11/formidable-forms
Length of output: 301
🏁 Script executed:
#!/bin/bash
# Check the test file to understand what setMethods() is doing
cat -n tests/phpunit/misc/test_FrmAddon.php | head -40Repository: Strategy11/formidable-forms
Length of output: 1335
🏁 Script executed:
#!/bin/bash
# Find phpunit configuration files
fd phpunitRepository: Strategy11/formidable-forms
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Check if there are any CI/workflow files that specify PHPUnit version
fd -e yml -e yaml | xargs grep -l phpunit 2>/dev/null | head -5Repository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check the phpunit.xml file
cat phpunit.xmlRepository: Strategy11/formidable-forms
Length of output: 949
🏁 Script executed:
#!/bin/bash
# Check bootstrap file for PHPUnit version hints
cat tests/phpunit/bootstrap.php | head -50Repository: Strategy11/formidable-forms
Length of output: 1028
🏁 Script executed:
#!/bin/bash
# Check GitHub workflows for PHPUnit version
fd -e yml -e yaml .github 2>/dev/null | xargs cat 2>/dev/null | grep -i phpunit -C 3 || echo "No workflows found"Repository: Strategy11/formidable-forms
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check if there's a .github/workflows directory or CI config
find . -type d -name '.github' -o -name '.gitlab-ci*' -o -name '.travis*' 2>/dev/null | head -5Repository: Strategy11/formidable-forms
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Check README or documentation for PHPUnit version requirements
find . -maxdepth 2 -type f \( -name 'README*' -o -name 'CONTRIBUTE*' -o -name 'DEV*' \) -exec head -50 {} \; 2>/dev/null | grep -i phpunit -C 2 || echo "No PHPUnit version info in docs"Repository: Strategy11/formidable-forms
Length of output: 102
🏁 Script executed:
#!/bin/bash
# List contents of .github directory
ls -la .github/Repository: Strategy11/formidable-forms
Length of output: 311
🏁 Script executed:
#!/bin/bash
# Check all YAML files in .github for PHPUnit mentions
find .github -type f -name '*.yml' -o -name '*.yaml' | xargs cat 2>/dev/nullRepository: Strategy11/formidable-forms
Length of output: 13279
Remove the no-op setMethods() call.
MockBuilder::setMethods() was deprecated in PHPUnit 8.3 and removed in PHPUnit 10.0. The call on lines 13–14 with no arguments is a no-op (equivalent to setMethods(null)) and should be removed entirely. This ensures compatibility with both current and future PHPUnit versions.
Current code:
$this->addon = $this->getMockBuilder( 'FrmTestAddon' )->setMethods()
->getMock();Remove ->setMethods() from the chain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/phpunit/misc/test_FrmAddon.php` around lines 13 - 14, The test
constructs a mock for FrmTestAddon using getMockBuilder but includes a no-op
call to setMethods() which is deprecated/removed; remove the ->setMethods() call
from the chain so the mock is built as
$this->getMockBuilder('FrmTestAddon')->getMock() (i.e., locate the
getMockBuilder('FrmTestAddon') call in tests/phpunit/misc/test_FrmAddon.php and
delete the setMethods() invocation).
stephywells
pushed a commit
that referenced
this pull request
Apr 4, 2026
…ver_tests Update rector config to actually fix issues in tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Looks like none of our Rector tests were working because it didn't identify our classes as PHPUnit class children.
Summary by CodeRabbit
Release Notes
Tests
Chores