Yoast/PHPUnit-Polyfills

[FR] Handle removed assertAttribute*() assertions

jrfnl opened this issue · 6 comments

jrfnl commented

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:

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 commented

@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 commented

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

@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 commented

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

jrfnl commented

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