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

add deprecation warning when called with just a class name #3860

Conversation

realFlowControl
Copy link
Sponsor Contributor

@realFlowControl realFlowControl commented Sep 9, 2019

This adds the deprecation warning when calling PHPUnit with just a class name or class name and filename as discussed in issue #3859

  • add deprecation warning
  • fix unit tests

@codecov
Copy link

codecov bot commented Sep 10, 2019

Codecov Report

Merging #3860 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3860      +/-   ##
============================================
+ Coverage      83.6%   83.62%   +0.01%     
- Complexity     3871     3875       +4     
============================================
  Files           151      151              
  Lines         10236    10241       +5     
============================================
+ Hits           8558     8564       +6     
+ Misses         1678     1677       -1
Impacted Files Coverage Δ Complexity Δ
src/TextUI/Command.php 74.95% <100%> (-0.46%) 217 <0> (+4)
src/Runner/StandardTestSuiteLoader.php 63.63% <0%> (+4.54%) 26% <0%> (ø) ⬇️
src/Util/FileLoader.php 100% <0%> (+10%) 8% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62cd277...155dae3. Read the comment docs.

@realFlowControl realFlowControl marked this pull request as ready for review September 10, 2019 18:25
@realFlowControl
Copy link
Sponsor Contributor Author

Hey @sebastianbergmann,
this is ready to be merged, i will prepare the removal than in another PR.
/Flo

@realFlowControl
Copy link
Sponsor Contributor Author

@sebastianbergmann should i rebase this on 8.4, so that we have the deprecation warning in 8 already?

@sebastianbergmann
Copy link
Owner

This branch has conflicts that must be resolved. The target branch should be master so that we get this for PHPUnit 8.5 (December 2019).

@realFlowControl
Copy link
Sponsor Contributor Author

I rebased on master and solved the conflict, everything green now 😄

@realFlowControl
Copy link
Sponsor Contributor Author

@sebastianbergmann had you time reviewing this?

@sebastianbergmann sebastianbergmann changed the base branch from master to 8.5 December 5, 2019 12:00
@sebastianbergmann sebastianbergmann added this to the PHPUnit 8.5 milestone Dec 5, 2019
@sebastianbergmann sebastianbergmann merged commit e506da1 into sebastianbergmann:8.5 Dec 5, 2019
@MaXal
Copy link

MaXal commented Dec 10, 2019

I'm from the PhpStorm team.
Could you please advise a new recommended way to run a single class or a single method?
We run a single test using:
phpunit --filter "/(::testMe)( .*)?$/" MyTest /Users/maxim.kolmakov/PhpstormProjects/untitled42/MyTest.php
and a class:
phpunit MyTest /Users/maxim.kolmakov/PhpstormProjects/untitled42/MyTest.php

This is quite an essential functionality that a lot of people are using every day and we really don't want to lose it!
image

@realFlowControl realFlowControl deleted the remove-class-name-cli-option branch December 10, 2019 12:26
@sebastianbergmann
Copy link
Owner

@MaXal Using --filter would be the "right way":

  • --filter MyTest will run all tests from MyTest
  • --filter MyTest::testMe will run the single test testMe from test class MyTest

Also note that we are not deprecating/removing the ability to invoke the PHPUnit test runner with the path to a test case class sourcecode file. We are "just" deprecating/removing the ability to invoke it with only a test case class name.

@MaXal
Copy link

MaXal commented Dec 11, 2019

@sebastianbergmann Thank you for the answer! We will update the line we're using to run tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants