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

[PhpUnitBridge] Added ClassExistsMock #28931

Merged
merged 1 commit into from Dec 10, 2018

Conversation

Projects
None yet
5 participants
@ro0NL
Copy link
Contributor

commented Oct 20, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#10528

I've thought about this before, and bumped into it again when trying to test #28898

This PR allows to mock class|interface|trait_exists to enable specific code path testing

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Oct 20, 2018

Mocks don't work with root-namespaced functions, and it turns out that *_exists() are in the list of the calls we recommend root-aliasing... Which means this will have a very low reach. For this reason, I'm not sure we should maintain this...

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2018

ouch :( i didnt saw any root namespaced variant in code base. Also CS does not propose these functions (yet) right?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Oct 20, 2018

Actually I was wrong sorry, *_exists() are not in the list!

@nicolas-grekas nicolas-grekas added this to the next milestone Oct 20, 2018

@ro0NL ro0NL referenced this pull request Oct 20, 2018

Merged

[Console] Add dumper #28898

@ro0NL ro0NL force-pushed the ro0NL:symbol-mock branch from 5cb3589 to 11442ea Oct 21, 2018

@ro0NL ro0NL changed the title [PhpUnitBridge] Added SymbolExistenceMock [PhpUnitBridge] Added ClassExistsMock Oct 21, 2018

@javiereguiluz

This comment has been minimized.

Copy link
Member

commented Nov 7, 2018

@ro0NL thanks for creating symfony/symfony-docs#10528 to track this feature in Symfony Docs. Much appreciated!

Sadly, the docs issue is not very actionable because the description of this PR is a bit lacking. Please update it to explain which changes would this require for full Symfony apps and for apps that use this as a stand-alone component.

If no changes are needed, please add some before/after examples or show what can we do now that wasn't possible before. Thanks!

Show resolved Hide resolved src/Symfony/Bridge/PhpUnit/CHANGELOG.md Outdated
@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2018

should there be any annotation integration, ie. @class-exists Some\Class Some\Other\Class?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Dec 1, 2018

I'm not sure - not if we don't have a use case.

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2018

#28898 could have one more test using @class-not-exists Symfony\Component\VarDumper\Dumper\CliDumper 👼

@fabpot

fabpot approved these changes Dec 10, 2018

@fabpot fabpot force-pushed the ro0NL:symbol-mock branch from 8e1a220 to 62caec1 Dec 10, 2018

@fabpot

This comment has been minimized.

Copy link
Member

commented Dec 10, 2018

Thank you @ro0NL.

@fabpot fabpot merged commit 62caec1 into symfony:master Dec 10, 2018

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Dec 10, 2018

feature #28931 [PhpUnitBridge] Added ClassExistsMock (ro0NL)
This PR was squashed before being merged into the 4.3-dev branch (closes #28931).

Discussion
----------

[PhpUnitBridge] Added ClassExistsMock

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#10528

I've thought about this before, and bumped into it again when trying to test #28898

This PR allows to mock `class|interface|trait_exists` to enable specific code path testing

Commits
-------

62caec1 [PhpUnitBridge] Added ClassExistsMock

@ro0NL ro0NL deleted the ro0NL:symbol-mock branch Dec 10, 2018

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.