Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Fix mocking of interface which extend Throwable #402

Conversation

DeepDiver1975
Copy link
Contributor

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/sebastianbergmann/phpunit/2995 branch from ed73d7e to 5c4a58f Compare February 7, 2018 19:03
@DeepDiver1975
Copy link
Contributor Author

@codecov-io
Copy link

codecov-io commented Feb 7, 2018

Codecov Report

Merging #402 into master will increase coverage by 0.17%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #402      +/-   ##
============================================
+ Coverage     72.77%   72.95%   +0.17%     
- Complexity      436      438       +2     
============================================
  Files            27       27              
  Lines          1227     1235       +8     
============================================
+ Hits            893      901       +8     
  Misses          334      334
Impacted Files Coverage Δ Complexity Δ
src/Generator.php 85.03% <100%> (+0.23%) 177 <0> (+2) ⬆️

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 0dbdc7c...e41ae82. Read the comment docs.

@DeepDiver1975 DeepDiver1975 changed the title Fixk mocking of interface which extend Throwable Fix mocking of interface which extend Throwable Feb 8, 2018
DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 12, 2018
DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 14, 2018
ownclouders pushed a commit to owncloud/core that referenced this pull request Feb 14, 2018
DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 16, 2018
DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 20, 2018
@DeepDiver1975
Copy link
Contributor Author

@sebastianbergmann can I drag your attention to this PR? ;-) THX a lot!

@sebastianbergmann
Copy link
Owner

Throwable is not supposed to be implemented in userland (I know that PHP allows it, which is unfortunate). I am not convinced that it would be "good" for PHPUnit to support this.

That being said, can you elaborate why you implement Throwable in your code instead of extending Exception?

@DeepDiver1975
Copy link
Contributor Author

That being said, can you elaborate why you implement Throwable in your code instead of extending Exception?

We are trying to mock dbal's Doctrine\DBAL\Driver\DriverException which is an interface.
https://github.com/doctrine/dbal/blob/2.6/lib/Doctrine/DBAL/Driver/DriverException.php#L32

See owncloud/core@1a5e980 - in the workaround I switched to Doctrine\DBAL\Driver\AbstractDriverException

@DeepDiver1975
Copy link
Contributor Author

FYI: Back in 2.5 the interface did not inherit from \Throwable - https://github.com/doctrine/dbal/blob/2.5/lib/Doctrine/DBAL/Driver/DriverException.php#L32

@sebastianbergmann
Copy link
Owner

@Ocramius @beberlei Can you explain why Doctrine does this? Thanks!

@Ocramius
Copy link

Throwable is not implemented in userland, it is just implemented as part of extends Exception:

interface SomeException extends Throwable // correct semantics, otherwise it can't be an exception anyway
{
}
class MyException extends OutOfBoundsException implements SomeException
{
}

The change is just semantic.

From my PoV, it makes no sense to mock a Throwable, as a collaborator will unlikely ever be a Throwable .

What PHPUnit would have to do (if mocking is needed) is extending Exception in the mock when Throwable is detected as implemented interface.

@Ocramius
Copy link

What PHPUnit would have to do (if mocking is needed) is extending Exception in the mock when Throwable is detected as implemented interface.

Ah, I see that this patch already does that :)

Also, re-thinking about the "makes no sense to mock a Throwable", it makes sense if the "auto-mocking on return types" kicks in on a foo() : Throwable

@sebastianbergmann
Copy link
Owner

Thank you for the explanation, @Ocramius.

@sebastianbergmann sebastianbergmann merged commit d2e92a4 into sebastianbergmann:master Feb 21, 2018
@DeepDiver1975 DeepDiver1975 deleted the bugfix/sebastianbergmann/phpunit/2995 branch February 21, 2018 12:36
@sebastianbergmann
Copy link
Owner

And thank you, @DeepDiver1975, for the patch! :-)

@DeepDiver1975
Copy link
Contributor Author

And thank you, @DeepDiver1975, for the patch! :-)

my pleasure - happy to contribute back to a tool we use for years!

DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 21, 2018
DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 22, 2018
DeepDiver1975 added a commit to owncloud/core that referenced this pull request Feb 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue in mocking Throwable with php7.0, 7.1 and 7.2
4 participants