One of the security patches for 3.8.12 adds an extra check in Joomla\CMS\Form\Form::validateField()
and makes use of an optional method parameter to do this. For the single legitimate call to this method in the production code, this assumption is OK to make, but it has broken our unit tests and potentially causes issues if someone has subclassed this class and calls the method omitting the optional $input
argument. Therefore, we must check the argument is provided before using it.
I think you're going to have to rely on the tests and code review for this one. There is not a use in this repo that can create this issue outside the unit test suite, and I'm not about to build an extension package to try and recreate this bug so it can be "easily" validated.
Yep.
To be honest though the tests that failed with this change aren't great tests in the first place (they're doing some janky stuff to directly test non-public APIs versus only testing the public API which indirectly would cover this method). But, the janky tests did expose a legitimate production issue with trying to use an optional argument without checking that it was actually given in the first place, so in this case it did help with something, but in a very unexpected way.
Status | New | ⇒ | Discussion |
Category | ⇒ | Unit Tests |
Status | Discussion | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-08-28 23:03:37 |
Closed_By | ⇒ | mbabker | |
Labels |
Added:
?
|
Category | Unit Tests | ⇒ | Libraries |
Maybe I am being daft but isn't the point of unit tests is that code changes shouldnt break the tests