[#25761] - [4.0] Adding further tests for Joomla\CMS\Table\Table::bind()
- Fixed in Code Base
- 26 Aug 2019
- Medium
- Build: 4.0-dev
- # 25761
- Diff
- Hackwar:j4tests_table
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
This adds further tests for Table::bind(). These tests uncovered 3 issues in that method:
- If neither an array nor an object is given to bind, the
onTableBeforeBindevent throws an exception, but not theInvalidArgumentExceptionthatbind()is supposed to throw. So the check for the "validity" of the input data is pulled to the front of the method. - Likewise the $ignore parameter. This can be both an array or a space separated string of field names. If you use that string variant, the events throw an error of invalid input.
- If a field is marked as JSON encoded, but the input is an object, this code will fail because it treats the object as an array.
I still have to find a way to test the events.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries Unit Tests |
| Labels |
Added:
?
?
|
||
I've extended the tests, but it fails on the event testing. If someone has an idea...
Found the issue and fixed it. I also cleaned up a few things...
Yah my guess is getTableColumns in the db driver is bugged. Should be able to write a test there too :P
| Category | Libraries Unit Tests | ⇒ | External Library Composer Change Libraries Unit Tests |
With the other PR merged, this one would then now be ready. Can you merge this one?
Or not. Now we have the same issue as in the backend template repo. So going to wait for that to be merged before we can merge this one. Giving me more time to write more tests. Maybe.
| Labels |
Added:
?
|
||
I've updated the composer stuff with just the updated database package and now the tests pass. Would be ready to merge this now.
Thanks
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-08-26 08:20:28 |
| Closed_By | ⇒ | HLeithner |
Number 3 must be an issue in staging I guess? because that's not new code for j4.