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

Fix "dynamic call to static method" phpstan error in tests #612

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

aivchen
Copy link
Contributor

@aivchen aivchen commented Jan 21, 2024

Pull Request

What does this PR do?

  • replaces $this->assertXXX calls with static::assertXXX
  • replaces assertEquals calls with assertSame
  • ordered arguments in assertSame calls

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@norkunas
Copy link
Collaborator

norkunas commented Jan 21, 2024

I'm not sure about this change, I think in phpunit v10 they are not static anymore,unless I am mistaken

Edit: yes I'm wrong 🙈

@aivchen
Copy link
Contributor Author

aivchen commented Jan 22, 2024

I ended up adding 2 php-cs-fixer rules:

  • php_unit_test_case_static_method_calls - to always have self/static calls to phpunit methods
  • php_unit_strict - to always have assertSame in tests

Copy link
Collaborator

@norkunas norkunas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I caught all cases where $expected and $actual params are swapped

tests/Contracts/CancelTasksQueryTest.php Outdated Show resolved Hide resolved
tests/Contracts/DeleteTasksQueryTest.php Outdated Show resolved Hide resolved
tests/Contracts/TasksQueryTest.php Outdated Show resolved Hide resolved
tests/Contracts/TasksQueryTest.php Outdated Show resolved Hide resolved
tests/Endpoints/ClientTest.php Outdated Show resolved Hide resolved
tests/Endpoints/IndexTest.php Outdated Show resolved Hide resolved
tests/Endpoints/IndexTest.php Outdated Show resolved Hide resolved
tests/Endpoints/IndexTest.php Outdated Show resolved Hide resolved
tests/Endpoints/IndexTest.php Outdated Show resolved Hide resolved
tests/Endpoints/IndexTest.php Outdated Show resolved Hide resolved
Co-authored-by: Tomas Norkūnas <norkunas.tom@gmail.com>
@norkunas
Copy link
Collaborator

bors merge

meili-bors bot added a commit that referenced this pull request Jan 23, 2024
612: Fix "dynamic call to static method" phpstan error in tests r=norkunas a=aivchen

# Pull Request

## What does this PR do?
- replaces `$this->assertXXX` calls with `static::assertXXX`
- replaces `assertEquals` calls with `assertSame`
- ordered arguments in `assertSame` calls
## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: andrew <and.ivchenkov@gmail.com>
Co-authored-by: Andrew Ivchenkov <and.ivchenkov@gmail.com>
Copy link
Contributor

meili-bors bot commented Jan 23, 2024

Build failed:

@norkunas
Copy link
Collaborator

bors merge

Copy link
Contributor

meili-bors bot commented Jan 23, 2024

@meili-bors meili-bors bot merged commit 7586fc9 into meilisearch:main Jan 23, 2024
9 checks passed
@norkunas
Copy link
Collaborator

Thank you @aivchen

@aivchen aivchen deleted the phpstan branch January 23, 2024 07:21
@curquiza curquiza added the maintenance Anything related to maintenance (CI, tests, refactoring...) label Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Anything related to maintenance (CI, tests, refactoring...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants