Feeling Lucky
?
?
Maintainers Checked
[#38095] - [4.2] Remove mocking XMLReader as it's bad practice. Fix parsing version
- Fixed in Code Base
- 24 Jun 2022
- Medium
- Build: 4.2-dev
- # 38095
- Diff
- wilsonge:xml_reader_tests
User tests: Successful: Unsuccessful:
Summary of Changes
Fixes tests to run on PHP 8.1. Generally we shouldn't be mocking core PHP classes so this fixes:
- Everywhere we mocked and then didn't even mock any values/return types etc. Just insert a real dummy instance
- The one place we did test was mocking versions. Here however this covered up a bug because when turning to concrete tests it turned out we parsed the XML version in the XML doc type rather than the version of the feed being used :D This bug is fixed as part of this and one extra test case in atom added
Testing Instructions
Check Drone 8.1 tests
Actual result BEFORE applying this Pull Request
All tests related to feed do not pass
Expected result AFTER applying this Pull Request
All tests related to feed do pass (note session tests do fail - but that will be the subject of a different PR)
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries Unit Tests |
HLeithner
- comment
- 19 Jun 2022
I mean a really test in an installed cms to merge this PR ;) not only codereview
wilsonge
- comment
- 19 Jun 2022
Took a 3rd party one in too for good measure
| Title |
|
||||||
| Labels |
Added:
?
?
Maintainers Checked
|
||
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-06-24 09:47:07 |
| Closed_By | ⇒ | HLeithner |
HLeithner
- comment
- 24 Jun 2022
Thanks
wilsonge
- comment
- 24 Jun 2022
Thankyou!
Agreed but probably outside the scope of this PR