Skip to content

Conversation

@alongosz
Copy link
Member

@alongosz alongosz commented Jun 4, 2025

🎫 Issue IBX-9727

Related PRs:

  • ibexa/product-catalog#1340

Description:

This PR fixes strict types for \Ibexa\Tests\Core\FieldType\BaseFieldTypeTestCase and its children.

There are a few interesting aspects of those changes.

  1. The line diff comes from the fact that around 26 test classes inherit from BaseFieldTypeTestCase which includes extensive documentation how to implement data provider. Those PHPDoc blocks were copied 1:1 from the base into the children. Now that I needed to fix return strict type in these blocks, I decided to remove the redundancy instead of fixing it everywhere as well.
  2. PC tests extend from these unit tests but they define better-shaped data provider override. Therefore, when adding strict return type and iterable shape, I needed to adjust core data providers themselves. Most of them now yield named data provider sets instead of returning them.
  3. There are some tests testing pre-strict-types validation. I've added inline phpstan-ignore <identifier> ignores as the sole purpose of those tests is to test what PHPStan reports as an error too. Once we get strict types to the actual field type implementations, those tests will be easily spotted and removed. However I'm not gonna do it in this PR for sanity reasons.
  4. Most of the field types inherit \Ibexa\Tests\Core\FieldType\FieldTypeTestCase which inherits \Ibexa\Tests\Core\FieldType\BaseFieldTypeTestCase. The only one directly ineritting \Ibexa\Tests\Core\FieldType\BaseFieldTypeTestCase is generic field type test. The only difference between the other core types and the generic one is that the latter one doesn't implement Comparable. That was reflected by keeping
private FieldType $fieldTypeUnderTest;

on BaseFieldTypeTestCase and applying covariance on FieldTypeTestCase in the form of:

private FieldType & Comparable $fieldTypeUnderTest;

as one of the tests calls \Ibexa\Contracts\Core\FieldType\Comparable::valuesEqual method.
Maybe generic field type should also implement comparable, but it's unclear to me how to make it in a clean way.

SonarQube duplication report is valid, but out of scope here.

For QA:

No QA required.

@alongosz alongosz force-pushed the tests-fix-core-ft-tests-strict-types branch from 24af71e to 8b3d21d Compare June 6, 2025 10:43
@alongosz alongosz force-pushed the tests-fix-core-ft-tests-strict-types branch from 8b3d21d to 3dab972 Compare June 6, 2025 12:35
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
4.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@alongosz alongosz marked this pull request as ready for review June 6, 2025 13:48
@alongosz alongosz requested a review from a team June 6, 2025 13:49
@ezrobot ezrobot requested review from Steveb-p, ViniTou, adamwojs, barw4, ciastektk, konradoboza, mikadamczyk, tbialcz and wiewiurdp and removed request for a team June 6, 2025 13:49
@adamwojs adamwojs merged commit cd53e7f into main Jun 6, 2025
25 of 26 checks passed
@adamwojs adamwojs deleted the tests-fix-core-ft-tests-strict-types branch June 6, 2025 14:21
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.

4 participants