Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixing up classes in some tests #8357

Closed
DanielBadura opened this issue Aug 2, 2022 · 9 comments
Closed

Mixing up classes in some tests #8357

DanielBadura opened this issue Aug 2, 2022 · 9 comments

Comments

@DanielBadura
Copy link
Contributor

Our CI build failed for auto upgrading our dependencies. In there psalm was updated from 4.24.0 to 4.25.0. Psalm now reports some strange errors like:

ERROR: UndefinedMethod - tests/Unit/Aggregate/AggregateRootTest.php:58:43 - Method Patchlevel\EventSourcing\Tests\Unit\Aggregate\AggregateRootTest::id does not exist (see https://psalm.dev/022)
        self::assertEquals($id, $profile->id());

Assuming that $profile is an instance of AggregateRootTest but it is instead an instance of AggregateRoot. See here for the code: https://github.com/patchlevel/event-sourcing/blob/2.1.x/tests/Unit/Aggregate/AggregateRootTest.php#L54-L58

Also here the full error list from our workflow: https://github.com/patchlevel/event-sourcing/runs/7616545968?check_suite_focus=true

Since the dependency update PR did update multiple deps, i tried it locally with only updating psalm with the same result.

I'm not sure whats going on if this is something on our side or broke with the new release. All errors are located in the tests folder and also only a tiny bit of these tests are affected.

@psalm-github-bot
Copy link

Hey @DanielBadura, can you reproduce the issue on https://psalm.dev ?

@AndrolGenhald
Copy link
Collaborator

@someniatko re #8249, care to take a look? Just from skimming through it I think this is a separate case from #8257.

@someniatko
Copy link
Contributor

@AndrolGenhald isn't #8335 supposed to fix the regression?

@AndrolGenhald
Copy link
Collaborator

@someniatko Sorry, forgot about that one, @DanielBadura could you test with 4.x-dev?

@someniatko
Copy link
Contributor

IMO 4.25.1 should be released with the regression fix, because it's already the second encounter of the issue.

@AndrolGenhald
Copy link
Collaborator

Agreed, I've got no idea how to do releases though so I'll just let @orklah handle it 🙂

@someniatko
Copy link
Contributor

p.s. if our suggestion is correct, it should be a duplicate of #8330

@DanielBadura
Copy link
Contributor Author

Can confirm that no issue occur with 4.x-dev 6998fab

@orklah
Copy link
Collaborator

orklah commented Aug 2, 2022

Released as 4.26.0 :)

@orklah orklah closed this as completed Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants