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

[FR] Handle removed assertAttribute*() assertions #2

Closed
jrfnl opened this issue Oct 26, 2020 · 6 comments
Closed

[FR] Handle removed assertAttribute*() assertions #2

jrfnl opened this issue Oct 26, 2020 · 6 comments

Comments

@jrfnl
Copy link
Collaborator

jrfnl commented Oct 26, 2020

A whole slew of `assertAttribute*() assertions were deprecated in PHPUnit 8.0.0 and removed in PHPUnit 9.0.0 without replacement.

The reasoning behind the removal was that it was too easy to test private/protected properties which should be considered implementation details and that those should not be tested directly.

Polyfilling this would require copying most of the removed code from PHPUnit, which I'm hesistant to do.

Refs:

@erickskrauch
Copy link

Hello. I'm interested in this feature. I would be happy to provide a pull request, but I don't quite understand the part of your "which I'm hesistant to do" message. Why don't you want to copy the code of PHPUnit?

@jrfnl
Copy link
Collaborator Author

jrfnl commented Nov 25, 2020

@erickskrauch Thanks for asking.

I don't quite understand the part of your "which I'm hesistant to do" message. Why don't you want to copy the code of PHPUnit?

There are two reasons why I'm not keen on this:

  1. I tend to agree with @sebastianbergmann's reasoning for removing the functionality.
  2. I don't think ripping off other people's code is the polite thing to do. Open source does not mean copyright doesn't apply.

Generally speaking, you'll find that the polyfills in this package use the underlying functionality in PHPUnit as-is without copying over large chunks of new/removed code.

All the same, I do recognize that refactoring the code or the tests of a package to work around the removal of the assertAttribute*() methods may take some time and that people, in the mean time, do want to use this library to allow for testing their code on PHP 8.0.

With this in mind, I've implemented some helper functions which will allow for testing private and protected attributes without duplicating the original functionality from PHPUnit. You can find the new functionality here: 45c6489

I'd be interested to hear if you think this is a workable enough compromise.

@jrfnl
Copy link
Collaborator Author

jrfnl commented Dec 1, 2020

@erickskrauch Just checking in to see whether you've had a chance to look at the alternative implementation.

@erickskrauch
Copy link

@jrfnl, hello. I'm sorry for the delay, right now have no possibility to work on a project, where I need this functionality. I hope I'll be able to return to it within a week.

@jrfnl
Copy link
Collaborator Author

jrfnl commented Dec 1, 2020

@erickskrauch Thanks for getting back to me. Let me know how you get on.

@jrfnl
Copy link
Collaborator Author

jrfnl commented Jun 3, 2021

Closing as fixed via the Helpers\AssertAttributeHelper as added in 45c6489

@jrfnl jrfnl closed this as completed Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants