Try using strict comparison in unit tests (prefer assertSame over assertEquals)#2905
Conversation
📝 WalkthroughWalkthroughAdded WordPress and PHPUnit linter integrations in configuration, tightened many PHPUnit test assertions from assertEquals to assertSame across tests, and updated the FrmFormsHelper::get_form_style docblock to indicate it may return int|string. No production logic or public API signatures changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/phpunit/fields/test_FrmFieldGridHelper.php.php (1)
1-1: Filename has double.phpextension.The file is named
test_FrmFieldGridHelper.php.phpwhich appears to be a typo. It should likely betest_FrmFieldGridHelper.php.
🤖 Fix all issues with AI agents
In `@tests/phpunit/forms/test_FrmFormsHelper.php`:
- Around line 100-101: Update the PHPUnit assertions that expect a string '1' to
expect the integer 1 so they match FrmFormsHelper::get_form_style()’s actual
return type; replace assertSame('1', FrmFormsHelper::get_form_style(...)) with
assertSame(1, FrmFormsHelper::get_form_style(...)) for the calls with null,
'default', and the array form without custom_style (and the other identical
assertion later in the file).
🧹 Nitpick comments (4)
tests/phpunit/entries/test_FrmTableHTMLGenerator.php (1)
55-58: UseassertSame()on line 58 for consistency with line 57.
remove_border()always returns a string viastr_replace(), so switching the second assertion toassertSamemaintains the strict-comparison approach consistently across both test cases.♻️ Suggested update
- $this->assertEquals( $table_generator->remove_border( $html, 'bottom' ), $html ); + $this->assertSame( $table_generator->remove_border( $html, 'bottom' ), $html );tests/phpunit/fields/test_FrmSubmitHelper.php (1)
18-18: SwapassertSameargument order to follow PHPUnit convention.PHPUnit expects
assertSame(expected, actual). Currently, the arguments are reversed—'Submit form'(the expected value) should come first, followed by$new_form->options['submit_value'](the actual value). This ensures failure messages display the correct diff.🔧 Proposed change
- $this->assertSame( $new_form->options['submit_value'], 'Submit form' ); + $this->assertSame( 'Submit form', $new_form->options['submit_value'] );tests/phpunit/misc/test_FrmAddon.php (1)
46-46: Consider converting remaining assertions for consistency.Lines 46 and 91 still use
assertEquals. For a complete migration to strict comparisons, these could also be converted:
- Line 46:
$license_keyis a string, soassertSamewould work.- Line 91:
$time['expected']is a boolean, soassertSamewould also be appropriate.tests/phpunit/fields/test_FrmFieldGridHelper.php.php (1)
77-77: Consider converting remainingassertEqualsfor consistency.Lines 77 and 147 still use
assertEqualsforcurrent_list_sizecomparisons. For a complete migration to strict comparisons, these could also be converted toassertSameif the property is always an integer.
…in_unit_tests Try using strict comparison in unit tests (prefer assertSame over assertEquals)
Summary by CodeRabbit
Tests
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.