Navigation Menu

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

willThrowException() only accepts Exception, not Throwable #3414

Closed
wants to merge 1 commit into from

Conversation

Stadly
Copy link
Contributor

@Stadly Stadly commented Nov 21, 2018

No description provided.

@gmponos
Copy link

gmponos commented Dec 15, 2018

I need this too.. any progress?

@epdenouden
Copy link
Contributor

Change looks useful. @Stadly can you merge in the latest master, so all tests run? Thx

@codecov
Copy link

codecov bot commented Dec 17, 2018

Codecov Report

Merging #3414 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3414      +/-   ##
============================================
- Coverage     82.15%   82.09%   -0.07%     
+ Complexity     3580     3530      -50     
============================================
  Files           143      143              
  Lines          9398     9291     -107     
============================================
- Hits           7721     7627      -94     
+ Misses         1677     1664      -13
Impacted Files Coverage Δ Complexity Δ
.../Framework/MockObject/Builder/InvocationMocker.php 80.82% <100%> (ø) 24 <1> (ø) ⬇️
src/Framework/Constraint/LogicalXor.php 36.11% <0%> (-58.34%) 14% <0%> (ø)
src/Util/Log/TeamCity.php 52.79% <0%> (-7.9%) 62% <0%> (+8%)
src/Util/TestDox/CliTestDoxPrinter.php 87.34% <0%> (-7.61%) 27% <0%> (-35%)
src/Util/Log/JUnit.php 90% <0%> (-4.45%) 32% <0%> (-3%)
src/Runner/PhptTestCase.php 79.06% <0%> (-1.11%) 79% <0%> (-4%)
src/Runner/TestSuiteSorter.php 99.02% <0%> (-0.98%) 47% <0%> (-7%)
src/Framework/Assert.php 92.13% <0%> (-0.3%) 232% <0%> (-8%)
src/TextUI/TestRunner.php 69.11% <0%> (-0.15%) 287% <0%> (-2%)
src/Util/Configuration.php 97.48% <0%> (-0.02%) 179% <0%> (-1%)
... and 8 more

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 a6d9814...ff7ae75. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 17, 2018

Codecov Report

Merging #3414 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3414   +/-   ##
=========================================
  Coverage     82.15%   82.15%           
  Complexity     3580     3580           
=========================================
  Files           143      143           
  Lines          9398     9398           
=========================================
  Hits           7721     7721           
  Misses         1677     1677
Impacted Files Coverage Δ Complexity Δ
.../Framework/MockObject/Builder/InvocationMocker.php 80.82% <100%> (ø) 24 <1> (ø) ⬇️

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 a6d9814...ff7ae75. Read the comment docs.

@Stadly
Copy link
Contributor Author

Stadly commented Dec 17, 2018

Done

@epdenouden
Copy link
Contributor

That's quick, thank you!

@epdenouden
Copy link
Contributor

@sebastianbergmann Change looks useful to me, would be nice to merge this, unless you know of something else breaking. The logging and reporting side uses Throwable already, too.

@gmponos
Copy link

gmponos commented Dec 17, 2018

@epdenouden I am not sure if with the current change other places must change as well...

I mean what about expectedException function.. this must also allow Throwables..

Haven't checked the code though... so it might already be fine...

@gmponos
Copy link

gmponos commented Dec 17, 2018

The logging and reporting side uses Throwable already, too

@epdenouden ah.. when you were saying reporting you were referring to things like expectedException?

@epdenouden
Copy link
Contributor

@gmponos That is a good point. I have not done an in-depth code review of the surrounding code. 🤐 Internally I think we can upgrade it all to Throwable anyway. Only exception would be strict checks for Exception, I think.

With the reporting I only meant the method signatures in the printers and listeners. Lots of stuff going to be refactored there and Throwable is the new fashionable thing. :)

@gmponos
Copy link

gmponos commented Dec 17, 2018

On the other hand you could accept current PR and see the other places later.. your call..

@epdenouden
Copy link
Contributor

Yes. The built-in collection of tests should keep us relatively safe 😂

@sebastianbergmann sebastianbergmann added type/bug Something is broken feature/test-doubles Stubs and Mock Objects labels Apr 22, 2019
@sebastianbergmann sebastianbergmann changed the title Make it possible to throw any Throwable willThrowException() only accepts Exception, not Throwable Apr 22, 2019
@sebastianbergmann
Copy link
Owner

Merged manually, thanks.

@Stadly Stadly deleted the patch-1 branch April 23, 2019 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-doubles Stubs and Mock Objects type/bug Something is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants