-
Notifications
You must be signed in to change notification settings - Fork 35
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
Implemented support for #[Test]
, #[DataProvider]
, #[Before]
attributes, cleaned up old PHPUnit/Prophecy support
#149
base: master
Are you sure you want to change the base?
Implemented support for #[Test]
, #[DataProvider]
, #[Before]
attributes, cleaned up old PHPUnit/Prophecy support
#149
Conversation
By removing `composer/package-versions-deprecated`, and instead using `composer-runtime-api:^2`, we both get rid of a dependency, and obtain better type information from our lookup. This also coincidentally drops a bunch of inline `@psalm-suppress` statements.
…PHPUnit version Since PHPUnit 8.0, `expectException()` has proper type annotations, and I even patched `createMock()` and similars so that proper generic types are inferred. These stubs are therefore no longer necessary. At this point, these classes are mostly "OK" in PHPUnit, starting ~8.5.1: * `PHPUnit\Framework\MockObject\Builder\InvocationMocker` (now declares missing types) * `PHPUnit\Framework\MockObject\MockObject` (still uses `@method` at class level, but that works) * `PHPUnit\Framework\MockObject\MockBuilder` (fully templated) * `PHPUnit\Framework\TestCase` (except for `prophesize()`, which was later removed)
Adding a conflict to PHPUnit earlier than 8.5.1, to make the phpunit version requirement more clear. Starting with PHPUnit 8.0, assertions have decent assertion metadata declarations on most methods. This also removes assertion tests, simplifying the test suite further.
Starting from Prophecy 1.20.0, methods have proper generic types declared. Adding a conflict to prevent earlier prophecy versions from being co-installed. Refs: * phpspec/prophecy@e4d39b1 * phpspec/prophecy@0bb033e * phpspec/prophecy@4506935
Starting from `phpspec/prophecy-phpunit:2.3.0`, the out-of-the-box `ProphecyTrait` is well typed. Ref: phpspec/prophecy-phpunit@ba6dc1b
Ah, I need to get the AST before a |
97f36b3
to
915026b
Compare
EDIT: outdated / done. @danog locally, everything seems green now: mostly spent time deleting code (good!). I'm hoping to make a second PR with the support for
|
#[Test]
, #[DataProvider]
, #[Before]
attributes, cleaned up old PHPUnit/Prophecy support
…deception-psalm-module` These packages are runtimes that invoke `psalm/plugin-phpunit` via `exec()`, without sharing codespace with it, and therefore are safe to externalize elsewhere. We can't afford keeping these dependencies co-located with the main `composer.json` / `composer.lock`, as they conflict with non-dev dependencies that we want to explicitly test against.
Patch is done, but it doesn't run due to Codeception toolinng not working. I raised psalm/codeception-psalm-module#54, but am still not happy with it. What I will likely do instead: will add Behat, will write a small context that replicates current steps, and will drop the external dependency. While it is indeed a bit aggressive (and good work was done here), the external dependency is too much for what we're testing here, and these scenario step runners should not be maintained outside this codebase. |
I will actually pursue the Behat approach: it is a single class to write, and will still keep it under |
While `psalm/codeception-psalm-module` is a good idea in theory, in practice, Gherkin scenario steps as a composer dependency are really (REALLY) hard to distribute and maintain together with a test suite. This move replaces the external dependency with a local one, with the test runner mostly staying out of the way, and test logic all implemented in the `Context` class we wrote ourselves. The main aim is to improve atomicity of changes on the test suite itself. Ref: psalm/codeception-psalm-module#54
I think it would be more beneficial to explore the solution within the current codeception infrastructures because it would help all plugins that already use |
I'm OK with such a decision, but I'm not going to go back to codeception myself :D |
Oh, it's not a decision, it's just my thoughts. I'm on hiatus, @danog makes decisions now. |
I initially wanted to introduce PHPUnit attribute support, but the amount of effort needed (mostly due to complex test tooling in this package) led me to removing everything that isn't strictly necessary anymore.
I've added
conflict
clauses for all old/unsupported versions of PHPUnit / Prophecy.I've yet to understand how to read an attribute via PHP-Parser: debugging with separate-process execution isn't properly "fun" :D
no longer needed for this patch2>&1
expression leads to all Psalm unrelatedSTDERR
breaking report parsing codeception-psalm-module#54 needed