-
Notifications
You must be signed in to change notification settings - Fork 17
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
Support for argument expectations in Stubs #26
base: master
Are you sure you want to change the base?
Conversation
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.
Really nice! LGTM.
Will wait for a bit before merging to give other reviewers a chance to review.
The notes are minor, hope you don't mind adding some docs where needed.
@SamMousa no problem, I'll also wait a bit for more feedback so I can fix it all in one go |
Added both requested PHPDocs |
* @param array ...$arguments | ||
* @return StubMarshaler | ||
*/ | ||
public function with(...$arguments) |
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.
This docblock is a good code documentation, but in order to be visible in documentation pages of https://codeception.com it must be documented elsewhere.
Reference page https://codeception.com/docs/reference/Stub is generated from the docblocks of Stub.php so please mention your new functionality in appropriate place of that file.
There are some examples of using Stub in https://codeception.com/docs/05-UnitTests#Test-Doubles
You can update it by editing https://github.com/Codeception/codeception.github.com/blob/master/docs/05-UnitTests.md
Your new functionality will only be available on PHP 7.2+ with PHPUnit 8.4 or 9, unless this change is merged to 2.x branch too.
To be honest, I'm a bit confused. What can be changed in this repo in order to fix those docs? I suppose not much? And it doesn't make sense to update the docs on the website without the functionality being present right? It seems https://codeception.com/docs/reference/Stub doesn't really have much documentation about Expected. Looks like it's all on https://codeception.com/docs/reference/Mock, so probably makes more sense to add it in there? As for https://codeception.com/docs/05-UnitTests#Test-Doubles, it's also not mentioning anything about Expected, but references the other pages, so I guess it's fine to leave that one as-is. Please suggest which way you'd like me to proceed so we can get this PR merged. On a side note, this PR fixes issue #32 |
Basic support for argument expectactions in Stubs, by chaining a with() method to Expected. This is something we were really missing compared to plain PHPUnit mocks, and turned out to be not that hard to add.
Syntax: Stub\Expected::once()->with('argument', 1, new StdClass)
Fully backwards compatible and unit tests have been added