fix(tests): Update attribute retrieval in tests to use getAttribute() method for consistency.#23
Conversation
…)` method for consistency.
📝 WalkthroughWalkthroughTests updated across the suite to read attribute values via the public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (26)
🚧 Files skipped from review as they are similar to previous changes (11)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-01-09T16:05:15.502ZApplied to files:
🔇 Additional comments (15)
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
tests/Global/HasDraggableTest.php (1)
84-95: Change default parameter fromnullto''in thegetAttributecall.The test data provider expects an empty string
''when the draggable attribute isnullor empty. The current call togetAttribute(GlobalAttribute::DRAGGABLE, null)at line 88 returnsnullas the default, causing a mismatch with the expected''value from the provider.Proposed fix
- $instance->getAttribute(GlobalAttribute::DRAGGABLE, null), + $instance->getAttribute(GlobalAttribute::DRAGGABLE, ''),tests/Global/HasTabIndexTest.php (1)
84-95: ChangegetAttribute()default fromnullto''to match test expectations.The data provider expects an empty string
''for unset/null tabindex cases (both 'null' and 'unset with null' scenarios), butgetAttribute(GlobalAttribute::TABINDEX, null)returnsnullinstead. This causes test assertion failures at line 86.Fix
- $instance->getAttribute(GlobalAttribute::TABINDEX, null), + $instance->getAttribute(GlobalAttribute::TABINDEX, ''),tests/Global/HasTranslateTest.php (1)
84-95: ChangegetAttribute()default parameter fromnullto''for consistency with test data providers.The test calls
getAttribute(GlobalAttribute::TRANSLATE, null)but theTranslateProviderexpects all results to be empty string (''), nevernull. All provider test cases return''for unset/null scenarios. Update the default parameter to match the expected values:Proposed fix
- $instance->getAttribute(GlobalAttribute::TRANSLATE, null), + $instance->getAttribute(GlobalAttribute::TRANSLATE, ''),This same pattern exists across other similar test files (HasAltTest, HasDecodingTest, HasHeightTest, HasHrefTest, HasReferrerpolicyTest, etc.), all using
nullas default where their providers expect''.tests/Global/HasContentEditableTest.php (1)
84-95: Update test default parameter to match changed getAttribute() behavior.The test data in ContentEditableProvider expects
getAttribute()to return an empty string''when thecontenteditableattribute is unset (e.g., whencontentEditable(null)is called). However, the externalui-awesome/html-helperpackage'sgetAttribute()method now returnsnullas the default instead of''.Fix
- $instance->getAttribute(GlobalAttribute::CONTENTEDITABLE, null), + $instance->getAttribute(GlobalAttribute::CONTENTEDITABLE, ''),This change aligns the test with the updated
getAttribute()behavior while maintaining the expected behavior verified in the ContentEditableProvider test data.tests/HasCrossoriginTest.php (1)
84-95: Change default value fromnullto empty string.The test data expects
getAttribute()to return an empty string when the attribute is unset, but passingnullas the default will returnnullinstead. The CrossoriginProvider test cases confirm this: both the 'null' and 'unset with null' cases expect an empty string''as the result.Regarding enum vs string keys: the pattern is consistent within each enum type by design—Attribute and ElementAttribute enums use
->value(string keys), while GlobalAttribute enums pass the enum directly. Both forms are supported bygetAttribute(), so this is a stylistic choice rather than an inconsistency requiring standardization.Proposed fix
- $instance->getAttribute(Attribute::CROSSORIGIN->value, null), + $instance->getAttribute(Attribute::CROSSORIGIN->value, ''),tests/Global/HasMicroDataTest.php (4)
132-156: Critical: Type mismatch causes test failures.The method signature expects
string $expectedValue(line 135), butgetAttribute(GlobalAttribute::ITEMID, null)returnsnullwhen the attribute is not set. This causes the pipeline failures shown in the logs.🔧 Required fixes
- Update the method signature to accept
null:public function testSetItemIdAttributeValue( string|null $itemId, array $attributes, - string $expectedValue, + string|null $expectedValue, string $expectedRenderAttribute, string $message, ): void {
- Ensure the data provider
ItemIdProvider::values()returnsnullinstead of''for cases where the attribute is not set.
162-186: Critical: Type mismatch causes test failures.The method signature expects
string $expectedValue(line 165), butgetAttribute(GlobalAttribute::ITEMPROP, null)returnsnullwhen the attribute is not set.🔧 Required fix
Update the method signature:
public function testSetItemPropAttributeValue( string|null $itemProp, array $attributes, - string $expectedValue, + string|null $expectedValue, string $expectedRenderAttribute, string $message, ): void {Ensure
ItemPropProvider::values()returnsnullinstead of''for missing attributes.
192-216: Critical: Type mismatch causes test failures.The method signature expects
string $expectedValue(line 195), butgetAttribute(GlobalAttribute::ITEMREF, null)returnsnullwhen the attribute is not set.🔧 Required fix
Update the method signature:
public function testSetItemRefAttributeValue( string|null $itemRef, array $attributes, - string $expectedValue, + string|null $expectedValue, string $expectedRenderAttribute, string $message, ): void {Ensure
ItemRefProvider::values()returnsnullinstead of''for missing attributes.
222-246: Critical: Type mismatch causes test failures.The method signature expects
bool|string $expectedValue(line 225), butgetAttribute(GlobalAttribute::ITEMSCOPE, null)returnsnullwhen the attribute is not set.🔧 Required fix
Update the method signature:
public function testSetItemScopeAttributeValue( bool|null $itemScope, array $attributes, - bool|string $expectedValue, + bool|string|null $expectedValue, string $expectedRenderAttribute, string $message, ): void {Ensure
ItemScopeProvider::values()returnsnullinstead of''for missing attributes.tests/Global/HasAccesskeyTest.php (1)
69-93: Critical: Type mismatch causes test failures.The method signature expects
string $expectedValue(line 72), butgetAttribute(GlobalAttribute::ACCESSKEY, null)returnsnullwhen the attribute is not set. This causes the pipeline failures.🔧 Required fix
Update the method signature:
public function testSetAccesskeyAttributeValue( string|null $accesskey, array $attributes, - string $expectedValue, + string|null $expectedValue, string $expectedRenderAttribute, string $message, ): void {Ensure
AccesskeyProvider::values()returnsnullinstead of''for cases where the attribute is not set.
🤖 Fix all issues with AI agents
In @tests/Element/HasAltTest.php:
- Around line 82-86: The failing tests are due to a default changing from '' to
null; update the AltProvider::values() data provider so any case that previously
expected an empty string for an unset ALT attribute now returns null instead;
locate the AltProvider::values() method and replace expected '' entries for "not
set" scenarios with null so HasAltTest (which asserts
getAttribute(ElementAttribute::ALT->value, null)) matches the new default.
In @tests/Element/HasHrefTest.php:
- Around line 82-86: The assertion calls
getAttribute(ElementAttribute::HREF->value, null) but tests expect an empty
string when href is unset; change the default parameter from null to '' in the
getAttribute call so it returns an empty string (i.e., replace
getAttribute(ElementAttribute::HREF->value, null) with
getAttribute(ElementAttribute::HREF->value, '') in the HasHrefTest assertions).
In @tests/Element/HasReferrerpolicyTest.php:
- Around line 85-89: The failing assertions are due to the
ReferrerpolicyProvider data provider expecting an empty string for cases where
getAttribute(..., null) should return null; update
ReferrerpolicyProvider::values so that the 'null' case and the 'unset with null'
case use null as the expected value instead of ''. Locate the
ReferrerpolicyProvider::values method and replace the expected '' entries for
those two cases with null so the test compares against null when the default is
null.
In @tests/Element/HasSrcTest.php:
- Around line 82-86: The failing tests are caused by the data provider returning
an empty string where the default attribute value is now null; update
SrcProvider::values() so any case representing an unset SRC attribute returns
null instead of '' (so the expected value matches the call to
getAttribute(ElementAttribute::SRC->value, null) in HasSrcTest::test...).
In @tests/Element/HasWidthTest.php:
- Line 84: The test failure is due to WidthProvider::values expecting an empty
string for unset width but getAttribute(ElementAttribute::WIDTH, null) returns
null; update the WidthProvider::values data provider to use null (not '') for
cases where the width attribute is not set or explicitly null so assertions in
HasWidthTest (the getAttribute call) match the actual return value.
In @tests/Global/HasClassTest.php:
- Line 92: Tests are failing because HasClassTest calls
$instance->getAttribute(GlobalAttribute::CLASS_CSS, null) which returns null in
cases where no class is set, but ClassProvider::values currently expects an
empty string; update ClassProvider::values to use null (not '') for those test
cases so expected values match getAttribute's null return, and ensure any test
cases referencing empty-class scenarios reference null for the CLASS_CSS
expectation.
In @tests/Global/HasDirTest.php:
- Around line 85-89: The test fails because the data provider still expects an
empty string for unset DIR attributes; update DirProvider::values() so any case
representing an unset GlobalAttribute::DIR returns null (not ''), ensuring the
expectedValue matches $instance->getAttribute(GlobalAttribute::DIR, null) in
HasDirTest::assertSame; change the specific entries in DirProvider::values()
that currently use '' to use null while keeping other test cases unchanged.
In @tests/Global/HasLangTest.php:
- Around line 85-89: The failing tests come from a mismatch where the default
for an unset LANG attribute changed from '' to null; update the data provider
LangProvider::values() so any case that previously expected an empty string ('')
for an unset GlobalAttribute::LANG now returns null instead, ensuring the test
assertion in HasLangTest::(the test using getAttribute(GlobalAttribute::LANG,
null)) compares against null. Locate LangProvider::values(), replace the
relevant '' entries for unset/absent LANG scenarios with null, and run the tests
to confirm the assertion self::assertSame($expectedValue,
$instance->getAttribute(GlobalAttribute::LANG, null)) now passes.
In @tests/Global/HasRoleTest.php:
- Line 87: The test data provider RoleProvider::values is expecting an empty
string for cases where the role attribute is unset, but
getAttribute(GlobalAttribute::ROLE, null) returns null; update
RoleProvider::values to use null (not '') for those test cases so the expected
value matches getAttribute's null result and fixes the failing assertions.
In @tests/Global/HasSpellcheckTest.php:
- Around line 85-89: The test is failing because
getAttribute(GlobalAttribute::SPELLCHECK, null) returns null while providers
still expect empty string; change the default parameter from null to '' in the
assertion (replace the second arg to getAttribute with ''), and sweep other
similar tests (e.g., HasRelTest, HasFetchpriorityTest, HasCrossoriginTest) to
update calls using getAttribute(..., null) back to getAttribute(..., '') so
providers like SpellcheckProvider continue to receive '' for unset attributes.
In @tests/Global/HasStyleTest.php:
- Line 88: Update the StyleProvider::values data provider so that cases
representing an unset or null style attribute use null instead of an empty
string; specifically change the expected value for tests that assert
$instance->getAttribute(GlobalAttribute::STYLE, null) (in HasStyleTest) from ''
to null so the provider matches getAttribute()'s null return when the style is
not set.
In @tests/Global/HasTitleTest.php:
- Around line 84-88: The assertion fails because getAttribute is called with a
null default, yielding null instead of the expected empty string; update the
call in HasTitleTest to use an empty-string default (change
getAttribute(GlobalAttribute::TITLE, null) to
getAttribute(GlobalAttribute::TITLE, '')) and apply the same change across other
tests that assert empty-string expectations (e.g., HasClass, HasTranslate,
HasTabIndex, HasStyle, HasSpellcheck, HasRole, HasLang, HasId, HasDraggable,
HasDir, HasContentEditable, HasAccesskey, CanBeHidden, CanBeAutofocus) so the
returned value matches the TitleProvider/test data.
In @tests/HasFetchpriorityTest.php:
- Around line 86-90: The assertion fails because
getAttribute(Attribute::FETCHPRIORITY->value, null) returns null when the
attribute is absent but the test data provider expects an empty string; update
the call to getAttribute(Attribute::FETCHPRIORITY->value, '') so the default
matches the provider. Apply the same change to other affected tests that use
getAttribute(..., null) (e.g., HasRelTest, HasCrossoriginTest) so all default
values expected by their providers are empty strings rather than null.
In @tests/HasRelTest.php:
- Around line 85-89: The failing tests come from a mismatch between the default
for Attribute::REL (now null) and the expected values in RelProvider::values();
update the data provider so any cases that previously expected an empty string
('') for unset REL now expect null instead. Edit RelProvider::values() to
replace the '' expected entries with null (so
HasRelTest::assertSame($expectedValue,
$instance->getAttribute(Attribute::REL->value, null), $message) will match), and
run the tests to confirm no other providers or assertions still expect '' for
unset REL.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
CHANGELOG.mdtests/Element/HasAltTest.phptests/Element/HasDecodingTest.phptests/Element/HasHeightTest.phptests/Element/HasHrefTest.phptests/Element/HasReferrerpolicyTest.phptests/Element/HasSrcTest.phptests/Element/HasWidthTest.phptests/Global/CanBeAutofocusTest.phptests/Global/CanBeHiddenTest.phptests/Global/HasAccesskeyTest.phptests/Global/HasClassTest.phptests/Global/HasContentEditableTest.phptests/Global/HasDirTest.phptests/Global/HasDraggableTest.phptests/Global/HasIdTest.phptests/Global/HasLangTest.phptests/Global/HasMicroDataTest.phptests/Global/HasRoleTest.phptests/Global/HasSpellcheckTest.phptests/Global/HasStyleTest.phptests/Global/HasTabIndexTest.phptests/Global/HasTitleTest.phptests/Global/HasTranslateTest.phptests/HasCrossoriginTest.phptests/HasFetchpriorityTest.phptests/HasRelTest.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T16:05:15.502Z
Learnt from: terabytesoftw
Repo: ui-awesome/html-attribute PR: 18
File: tests/Support/Provider/Global/ContentEditableProvider.php:32-33
Timestamp: 2026-01-09T16:05:15.502Z
Learning: Preserve the original copyright year in file headers when moving PHP source files between packages within the ui-awesome repositories (e.g., from ui-awesome/html-core to ui-awesome/html-attribute). Do not update the header year to the current year; keep the original creation year as stated in the header. This applies to all PHP files throughout the repo that are moved between packages.
Applied to files:
tests/Global/HasMicroDataTest.phptests/Global/HasTabIndexTest.phptests/Element/HasHeightTest.phptests/Element/HasDecodingTest.phptests/Global/HasClassTest.phptests/Element/HasHrefTest.phptests/Element/HasAltTest.phptests/HasRelTest.phptests/Global/HasTitleTest.phptests/Global/HasRoleTest.phptests/HasFetchpriorityTest.phptests/Global/HasTranslateTest.phptests/Global/HasSpellcheckTest.phptests/Global/HasDraggableTest.phptests/Global/HasStyleTest.phptests/Element/HasWidthTest.phptests/Element/HasReferrerpolicyTest.phptests/Global/HasLangTest.phptests/Global/HasIdTest.phptests/HasCrossoriginTest.phptests/Global/CanBeHiddenTest.phptests/Element/HasSrcTest.phptests/Global/HasDirTest.phptests/Global/HasAccesskeyTest.phptests/Global/HasContentEditableTest.phptests/Global/CanBeAutofocusTest.php
🪛 GitHub Actions: build
tests/Global/HasMicroDataTest.php
[error] 146-146: Failed asserting that null is identical to ''.
[error] 146-146: Failed asserting that null is identical to ''.
[error] 176-176: Failed asserting that null is identical to ''.
[error] 176-176: Failed asserting that null is identical to ''.
[error] 206-206: Failed asserting that null is identical to ''.
[error] 206-206: Failed asserting that null is identical to ''.
[error] 236-236: Failed asserting that null is identical to ''.
[error] 236-236: Failed asserting that null is identical to ''.
[error] 266-266: Failed asserting that null is identical to ''.
[error] 266-266: Failed asserting that null is identical to ''.
tests/Global/HasTabIndexTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Element/HasHeightTest.php
[error] 82-82: Failed asserting that null is identical to ''.
[error] 82-82: Failed asserting that null is identical to ''.
tests/Element/HasDecodingTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Global/HasClassTest.php
[error] 90-90: Failed asserting that null is identical to ''.
[error] 90-90: Failed asserting that null is identical to ''.
[error] 90-90: Failed asserting that null is identical to ''.
tests/Element/HasHrefTest.php
[error] 82-82: Failed asserting that null is identical to ''.
[error] 82-82: Failed asserting that null is identical to ''.
tests/Element/HasAltTest.php
[error] 82-82: Failed asserting that null is identical to ''.
[error] 82-82: Failed asserting that null is identical to ''.
tests/HasRelTest.php
[error] 85-85: Failed asserting that null is identical to ''.
[error] 85-85: Failed asserting that null is identical to ''.
tests/Global/HasTitleTest.php
[error] 84-84: Failed asserting that null is identical to ''.
[error] 84-84: Failed asserting that null is identical to ''.
tests/Global/HasRoleTest.php
[error] 85-85: Failed asserting that null is identical to ''.
[error] 85-85: Failed asserting that null is identical to ''.
tests/HasFetchpriorityTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Global/HasTranslateTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Global/HasSpellcheckTest.php
[error] 85-85: Failed asserting that null is identical to ''.
[error] 85-85: Failed asserting that null is identical to ''.
tests/Global/HasDraggableTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Global/HasStyleTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Element/HasWidthTest.php
[error] 82-82: Failed asserting that null is identical to ''.
[error] 82-82: Failed asserting that null is identical to ''.
tests/Element/HasReferrerpolicyTest.php
[error] 85-85: Failed asserting that null is identical to ''.
[error] 85-85: Failed asserting that null is identical to ''.
tests/Global/HasLangTest.php
[error] 85-85: Failed asserting that null is identical to ''.
[error] 85-85: Failed asserting that null is identical to ''.
tests/Global/HasIdTest.php
[error] 82-82: Failed asserting that null is identical to ''.
[error] 82-82: Failed asserting that null is identical to ''.
tests/HasCrossoriginTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
tests/Element/HasSrcTest.php
[error] 82-82: Failed asserting that null is identical to ''.
[error] 82-82: Failed asserting that null is identical to ''.
tests/Global/HasDirTest.php
[error] 85-85: Failed asserting that null is identical to ''.
[error] 85-85: Failed asserting that null is identical to ''.
tests/Global/HasAccesskeyTest.php
[error] 83-83: Failed asserting that null is identical to ''.
[error] 83-83: Failed asserting that null is identical to ''.
tests/Global/HasContentEditableTest.php
[error] 86-86: Failed asserting that null is identical to ''.
[error] 86-86: Failed asserting that null is identical to ''.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (6)
tests/Global/CanBeAutofocusTest.php (1)
83-87: No action required. TheAutofocusProvidertest data correctly aligns with the test expectations. All test cases explicitly set the autofocus attribute with a boolean value before callinggetAttribute(), ensuring the returned value matches the expected boolean type. Thenulldefault parameter is not tested in this method because the attribute is always present.CHANGELOG.md (1)
14-14: CHANGELOG entry is accurate.The entry correctly describes the change to use
getAttribute()method for consistency in tests. No changes needed to this CHANGELOG line.tests/Element/HasHeightTest.php (1)
82-86: Incorrect review comment: Data provider expectations are correct and consistent across similar tests.The data provider for Height (like Width and Decoding) correctly expects an empty string
''when the attribute is set tonull. This is the intended behavior and is consistent across all similar attribute test data providers in the codebase.The actual issue appears to be unrelated to the data provider expectations: HasHeightTest uses
getAttribute(ElementAttribute::HEIGHT->value, null)while HasWidthTest usesgetAttribute(ElementAttribute::WIDTH, null)— note the inconsistent use of->value. This inconsistency, not the provider expectations, warrants investigation.Likely an incorrect or invalid review comment.
tests/Global/CanBeHiddenTest.php (1)
84-84: No action required; HiddenProvider is correct.The test always sets the
hiddenattribute viahidden($value)before callinggetAttribute(). All HiddenProvider test cases correctly expect the actual boolean value (falseortrue) that was set, notnull. The data provider is properly aligned with the code change.tests/Global/HasIdTest.php (1)
82-86: Fix broken test: replace undefined variables with actual valuesThe test has incomplete placeholder variables (
$expectedValueand$message) that need proper values. SincegetAttribute(GlobalAttribute::ID, null)correctly returnsnullwhen the ID is not set (viagetId() ?? null), the test should expect:- $expectedValue, + null, $instance->getAttribute(GlobalAttribute::ID, null), - $message, + 'ID attribute should return null when not set and null default provided',The implementation's null semantics are correct: the method properly returns
nullwhen both the ID provider and the default value arenull.Likely an incorrect or invalid review comment.
tests/Global/HasMicroDataTest.php (1)
252-276: The review comment is incorrect. The test method signature at lines 252-276 is correctly typed.The
ItemTypeProvider::values()test data consistently returns string values (either empty string''or URL strings) as$expectedValue, nevernull. The PHPStan return type annotation confirms this:array<string, array{string|null, mixed[], string, string, string}>, where position 3 (the expected value) isstring.Each test case returns:
- 'empty string':
''- 'null' input: expects
''(not null)- 'replace existing': expects the URL string
- 'string': expects the URL string
- 'unset with null': expects
''(not null)The parameter type
string $expectedValueis correct and matches the provider's type annotation. UnlikeItemScopeProviderwhich usesbool|stringfor its$expectedValueparameter (to handle boolean and string returns),ItemTypeProvideralways returns strings, makingstring $expectedValuethe appropriate type.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Pull Request
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.