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

Overhaul thrown exceptions #191

Merged
merged 5 commits into from
Dec 29, 2022

Conversation

unfulvio-godaddy
Copy link
Member

@unfulvio-godaddy unfulvio-godaddy commented Dec 26, 2022

Summary

Adds a WP_MockException object and overhauls exceptions used or referenced within project.

After consideration, I decided to go for more specific exceptions in the PHP stdlib - such as RunTimeException or InvalidArgumentException when applicable. I realized that WP_Mock already uses other domain-specific exceptions when it comes to assertions failed or mockery exceptions, so perhaps at the moment we don't need a specific WP_Mock exception.

Had to make additional changes due to PhpStan errors after updating code.

Closes: #189

Contributor checklist

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly
  • I have added tests to cover changes introduced by this pull request
  • All new and existing tests pass

Testing

  • Tests and code analysis passes

Reviewer checklist

  • Code changes review
  • Documentation changes review
  • Unit tests pass
  • Static analysis passes

@unfulvio-godaddy unfulvio-godaddy self-assigned this Dec 26, 2022
@unfulvio-godaddy unfulvio-godaddy requested a review from a team December 26, 2022 03:19
@coveralls
Copy link

coveralls commented Dec 26, 2022

Coverage Status

Coverage: 11.869% (+0.7%) from 11.201% when pulling fe8e5c3 on issue-189/introduce-wp-mock-exception into cdac012 on trunk.

Copy link
Contributor

@agibson-godaddy agibson-godaddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this because it seems fine, but I did leave a couple comments for you to consider.

php/WP_Mock/Exceptions/WP_MockException.php Outdated Show resolved Hide resolved
php/WP_Mock/Exceptions/WP_MockException.php Outdated Show resolved Hide resolved
@unfulvio-godaddy unfulvio-godaddy requested review from wvega-godaddy and removed request for wvega-godaddy December 29, 2022 01:27
@unfulvio-godaddy unfulvio-godaddy changed the title Introduce WP_MockException Overhaul thrown exceptions Dec 29, 2022
@unfulvio-godaddy unfulvio-godaddy requested review from agibson-godaddy and a team December 29, 2022 02:10
@unfulvio-godaddy
Copy link
Member Author

@agibson-godaddy sorry I went to a different direction after reading your comments but mostly re-evaluating the purpose of this PR - in the end maybe WP_Mock is already using some specific exceptions depending on context (from PhpUnit expectation failed exception, mockery exceptions, etc.). I have added a couple of InvalidArgumentException or RunTimeException where it seemed to make most sense. Perhaps there is no need to use a generic exception at the moment.

@agibson-godaddy agibson-godaddy merged commit 5733848 into trunk Dec 29, 2022
@agibson-godaddy agibson-godaddy deleted the issue-189/introduce-wp-mock-exception branch December 29, 2022 11:09
@jeffpaul jeffpaul added this to the 0.6.0 milestone Jan 4, 2023
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.

WP_Mock should define its own exception objects
4 participants