-
Notifications
You must be signed in to change notification settings - Fork 46
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
Assertion class as parameter for Codeception config #65
base: 1.1.x
Are you sure you want to change the base?
Conversation
@@ -20,10 +20,14 @@ services: | |||
- phpstan.typeSpecifier.functionTypeSpecifyingExtension | |||
- | |||
class: PHPStan\Type\PHPUnit\Assert\AssertMethodTypeSpecifyingExtension | |||
arguments: | |||
classWithAssertionMethods: PHPUnit\Framework\TestCase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've retained the class used in https://github.com/phpstan/phpstan-phpunit/pull/65/files#diff-afc1ebcbfd95543266d01a244f0e43deL27, but shouldn't this be PHPUnit\Framework\Assert
too?
bda246e
to
5af462c
Compare
Hi, I don't know anything about Codeception. Do you typically use Anyway, we could register everything by default (there's no downside) and just mention in the README that this extension works well with Codeception out of the box. WDYT? |
It was my wish from the beginning, but it not feasible. Codeception ships with 2 different types of test, the Unit one which is a subclass of What's the blocking? The giant class created lies under the project folder tree, and of course to be correctly autoloaded it must have the project's namespace, not a generic one. |
I feel like this is related to #103 I don't exactly see the benefit to parametrize the assertion class instead of just fixing the FQCN. But I'm not familiar at all with codeception. Can you provide a link to that I tried searching https://github.com/Codeception/phpunit-wrapper/tree/9.0/src but I didn't find it. |
I doesn't exist, Codeception doesn't work this way. There is no way |
The thing is making the class parametrizable will fix your use case by breaking PHP Unit analysis. I think a better approach would be to create another package dedicated to codeception, dependant on this one. This way you can have two instance of the extension working at the same time. |
May I ask you how this break would happen?
This PR already enable to have both PHPUnit and Codeception working in the same time, where do you see this isn't the case? |
🤦 You are right I didn't understand the PR correctly. I somehow thought you were just feeding the existing service with another default value. But you are in fact creating other instances with different parameters. This should work fine. 👍 |
Resolves #34