8000 [RFC][5.0][PhpunitBridge] deprecate/remove simple-phpunit · Issue #31948 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[RFC][5.0][PhpunitBridge] deprecate/remove simple-phpunit #31948

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.

Sign up for GitHub

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

Jump to bottom
Closed
smoench opened this issue Jun 8, 2019 · 16 comments
Closed

[RFC][5.0][PhpunitBridge] deprecate/remove simple-phpunit #31948

smoench opened this issue Jun 8, 2019 · 16 comments
Labels
Deprecation PhpUnitBridge RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@smoench
Copy link
Contributor
smoench commented Jun 8, 2019

Description
With version 4.8 phpunit had required the yaml component but starting with 5.0 they didn't anymore. The phpunit-bridge documentation still says there will be issues: Provides a modified version of PHPUnit that does not embed symfony/yaml nor prophecy to prevent any conflicts with these dependencies.

My prosposal would be deprecate simple-phpunit with 4.4 and remove it with version 5.0. simple-phpunit still has its issues (no extentions possible), lacks behind the latest phpunit versions and the arguments in the documentation are not valid anymore. In the documentation the official installation guide should be referenced.

If simple-phpunit is still required for (symfony) development I would recommend to extract this to another repository. But for end-users it is not really convenient.
@smoench smoench mentioned this issue Jun 8, 2019
Closed
@javiereguiluz javiereguiluz added PhpUnitBridge RFC RFC = Request For Comments (proposals about features that you want to be discussed) Deprecation labels Jun 8, 2019
@alexander-schranz
Copy link
Contributor

The simple-phpunit does not only handle the dependencies for phpunit it also register the symfony tests listener 8000 which enables the features listed in https://symfony.com/doc/current/components/phpunit_bridge.html if I'm not wrong, but think @nicolas-grekas know more about it.

@wouterj
Copy link
Member
wouterj commented Jun 12, 2019 •
edited
Loading

The Prophecy conflict is still true (e.g. https://github.com/symfony/property-info/blob/master/composer.json#L37)

Nvm :)

@stof
Copy link
Member
stof commented Jun 12, 2019

@wouterj no, because Prophecy supports newer versions of the phpdocumentor library now (since years). So composer would be able to find a matching package.

@wouterj
Copy link
Member
wouterj commented Jun 12, 2019 •
edited
Loading

Alrighty, if there are no conflicts anymore, 👍 to use normal PHPunit. (IIRC, the only thing Simple PHPunit provided was removing conflicting deps)

@stof
Copy link
Member
stof commented Jun 12, 2019

The remaining feature of simple-phpunit is the automatic registration of test listeners (for the time mocking and so on).
In a bare phpunit usage, you would need to register them explicitly in the phpunit config file (the deprecation handler still works out of the box)

@smoench
Copy link
Contributor Author
smoench commented Jun 12, 2019 •
edited
Loading

@alexander-schranz @stof via recipes we could provide a phpunit template where those listener are enabled by default.

@alexander-schranz
Copy link
Contributor

@smoench 👍 sounds good for me, so its also not hidden :)

@magnetik
Copy link
Contributor

I'm all in favor as it has confused some junior dev a lot:

  • Why is it called "simple-phpunit" ;
  • Why isn't phpunit available as standard dependency in my vendor directory before I started the test (So no auto completion in the IDE) ;
  • What version will be used etc.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 13, 2019 •
edited
Loading

Big 👎 on my side.
Adding phpunit in composer.json is a very bad practice, despite the fact the vast majority does it.
Sebastian Bergman expressed the same opinion several times if that matters.

@smoench
Copy link
Contributor Author
smoench commented Jun 14, 2019

@nicolas-grekas this RFC is not about enforcing a bad practice but improving PHPUnit's DX. Instead of doing it our way both communities should come together, improving the DX and providing a guide.

IMHO as long as PHPUnit (respectively @sebastianbergmann) is telling us it is installable via composer the vast majority will do it.
On the other hand the test classes are depending on PHPUnit sources so there is a dependency which should be defined in the composer.json. This could be stub sources for having autocompletion and static analysis.

We all have great knowledge about software developing and testing. Let's come together and improving the DX.

@nicolas-grekas
Copy link
Member

@smoench sure, I'm only giving my pov on the initial question: it's way too early to even consider deprecating the bridge. If some of us want to improve the state of things up to the point where no remaining features of the bridge need the simple-phpunit script, fine. But that's not the case right now. Please dig deeper into the set of features. I'd be happy to be wrong by mean of future contributions.

@ro0NL ro0NL mentioned this issue Jun 15, 2019
Merged
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 15, 2019 •
edited
Loading

I created symfony/symfony-docs#11757 to clarify what the script does. Note that the excerpt in the description of this issue is misleading: the doc already tells about more things the script provides.

If that was the original motivation (if not, what's the problem this issue wants to tackle?), I agree that using composer to control the version of phpunit to use in an app would be nice, but having a single pool of dependencies for the app + the tooling is really bad practice. phpstan-shim is another try at solving this issue. I've previously discussed this with some people: it would be great to make composer able to deal with several sets of dependencies, see e.g. composer/composer#6856 (comment)

But clearly, deprecating the script without any credible alternative is doing things the reverse way.

@ro0NL
Copy link
Contributor
ro0NL commented Jun 16, 2019 •
edited
Loading

Personally i appreciate simple-phpunit for being available, it allows to rely on a single package and have an operating + extended + autoloadable phpunit up&running.

💯 agree with @nicolas-grekas for our ultimate goal being to decouple tools from the project deps.

Also work is done to install simple-phpunit in various environments; https://hub.docker.com/r/jakzal/phpqa/ (jakzal/toolbox#35) I think we already reached a point of no return.

Nevertheless, if the wrapper can be simplified/cleaned up from redundant checks that's nice.

@smoench
Copy link
Contributor Author
smoench commented Jun 17, 2019

@nicolas-grekas Don't get me wrong. I don't want deprecate phpunit-bridge but simple-phpunit. IMHO simple-phpunit is a script respectively a tool and should not be part of a bridge. phpunit-bridge contains many great features / extensions for phpunit. They could be used independently from simple-phpunit. PHPUnit provides a scoped phar-file which should be the preferred one anyway. A big disadvantage of simple-phpunit it could not be extended with more great PHPUnit extensions.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 17, 2019 •
edited
Loading

Sure, I edited the word "bridge" and replaced it by "script", because this is really what I was talking about.

@derrabus derrabus mentioned this issue Jul 7, 2019
Closed
@nicolas-grekas
Copy link
Member

This won't happen without a credible alternative. I'm therefore closing as this is not actionable. Of course, any steps that could help migrate features provided by the script to e.g. phpunit would be good I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Deprecation PhpUnitBridge RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@javiereguiluz @smoench @nicolas-grekas @magnetik @stof @wouterj @ro0NL @alexander-schranz
0