Skip to content

fix tests after Symfony PropertyInfo update#1973

Merged
gechetspr merged 1 commit intopropelorm:masterfrom
perplorm:tests/change_in_symfony_PropertyInfo_component
Sep 4, 2023
Merged

fix tests after Symfony PropertyInfo update#1973
gechetspr merged 1 commit intopropelorm:masterfrom
perplorm:tests/change_in_symfony_PropertyInfo_component

Conversation

@mringler
Copy link
Contributor

@mringler mringler commented Jul 11, 2023

With the current Symfony components, one test in the 5-max and and 6-max group fails, see for example this run.

Apparently, the Symfony PropertyInfo has changed internally, and now an assertion fails. A comment in the test acknowledges that it tests Symfony internals:

        $fictionMetadatas = $fictionMetadata->getPropertyMetadata('isbn');

        // 1st is for ValidateTriggerFiction (base)
        // 2nd is for ValidateTriggerBook (base)
        // I'm not sure if this is needed. We should not care about validator internals
        $this->assertCount(2, $fictionMetadatas);

Examining $fictionMetadatas, I can see that both ValidateTriggerFiction and ValidateTriggerBook are present in $fictionMetadatas[0].constraintsByGroup:

Screenshot_2023-07-11_15-42-15

This is also successfully tested in the rest of the test case.

So apparently the PropertyInfo component now groups those constraints in the same PropertyMetadata object instead of using a new object for each. But this does not concern Propel. So we can just remove that assertion. We could also turn it into assertNotEmpty(), but that is tested implicitly in the next assertion.

@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2023

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.25%. Comparing base (011aaa7) to head (d2607c2).
⚠️ Report is 87 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1973      +/-   ##
============================================
+ Coverage     88.56%   89.25%   +0.69%     
  Complexity     8051     8051              
============================================
  Files           243      232      -11     
  Lines         24590    24544      -46     
============================================
+ Hits          21777    21906     +129     
+ Misses         2813     2638     -175     
Flag Coverage Δ
5-max 89.25% <ø> (+0.69%) ⬆️
7.4 89.25% <ø> (+0.69%) ⬆️
agnostic 67.44% <ø> (+0.14%) ⬆️
mysql 69.78% <ø> (+0.69%) ⬆️
pgsql 69.81% <ø> (+0.70%) ⬆️
sqlite 67.77% <ø> (+0.69%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@floriankraemer
Copy link

I'll get this merged, it makes sense.

@gechetspr gechetspr merged commit b9dee04 into propelorm:master Sep 4, 2023
@mringler mringler deleted the tests/change_in_symfony_PropertyInfo_component branch September 29, 2023 22:13
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