[4.2 RC1] Feed display module not working

[conseilgouz]

Pull Request for Issue #38310 .

Testing Instructions

Create a Feed display module.
Enter https://www.joomla.org/announcements.feed?type=rss in Feed URL parameter
Enter Sidebar-right as position (using cassiopeia template)

Actual result BEFORE applying this Pull Request

Feed display module shows "Feed not found" error in all cases.

Expected result AFTER applying this Pull Request

Display Feed infos.

[conseilgouz]

AppVeyor failed because $this->version has correct values for Atom and RSS files.

tests\Unit\Libraries\Cms\Feed\Parser\AtomParserTest.php file and tests\Unit\Libraries\Cms\Feed\Parser\RssParserTest.php seem to be wrong.

[richard67]

AppVeyor failed because $this->version has correct values for Atom and RSS files.

tests\Unit\Libraries\Cms\Feed\Parser\AtomParserTest.php file and tests\Unit\Libraries\Cms\Feed\Parser\RssParserTest.php seem to be wrong.

@conseilgouz Then you should fix the tests, too. Let us know if you need some help or advise for that.

[richard67]

P.S.: You can run the unit test locally on a git clone by running composer install (if not done yet) and then ./libraries/vendor/bin/phpunit --testsuite Unit from the Joomla folder.

[richard67]

@conseilgouz The failing tests can be found here https://github.com/joomla/joomla-cms/blob/4.2-dev/tests/Unit/Libraries/Cms/Feed/Parser/AtomParserTest.php#L374-L387 and here https://github.com/joomla/joomla-cms/blob/4.2-dev/tests/Unit/Libraries/Cms/Feed/Parser/RssParserTest.php#L583-L601 .

But from reading the code, the tests seem to be correct regarding the checked versions.

Here the output from the test run in Appveyor:

There were 2 failures:
1) Joomla\Tests\Unit\Libraries\Cms\Feed\Parser\AtomParserTest::testInitialiseSetsOldVersion
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'0.3'
+'1.0'
C:\projects\joomla-cms\tests\Unit\Libraries\Cms\Feed\Parser\AtomParserTest.php:386
2) Joomla\Tests\Unit\Libraries\Cms\Feed\Parser\RssParserTest::testParseSetsVersion
Failed asserting that null matches expected '2.0'.
C:\projects\joomla-cms\tests\Unit\Libraries\Cms\Feed\Parser\RssParserTest.php:600
FAILURES!
Tests: 757, Assertions: 1190, Failures: 2, Skipped: 1.
Command exited with code 1

So it seems your PR is not right or changes something at the wrong place.

[conseilgouz]

@richard67 : unit test files (RSS & ATOM) have been updated, so they match FeedFactory behaviour : go to root node and read it.

[conseilgouz]

@richard67 : integration/drone/pr has a phpmin-system-postgress error : I'm afraid I don't have enough knowledge to fix this one....

[conseilgouz]

After merging branch, postgress error disappeared...

[richard67]

@conseilgouz Sometimes we have unrelated errors in system tests e.g. due to timeouts.

But besides this I am not convinced of your changes in the tests. The tests looked right to me before. I don’t have the time now for testing it myself, but if nobody else can reproduce the issue I have doubts on this PR.

[conseilgouz]

@richard67 : about the changes, when initialise function is called in RssParser.php and AtomParser, stream (xmlreader) has read the root node : it's done in FeedFactory.php, function getFeed(), lines 68 and above.
In the unit test files, I just reproduced this behaviour.
Before my changes, initialise (in AtomParser.php and RssParser.php) was performing a moveToNextElement when reader was already on the feed or rss record (which should contain version info) : that was introduced by PR #38095

[conseilgouz]

but if nobody else can reproduce the issue I have doubts on this PR.

That's a 4.2 RC1 issue. Let's hope somebody else will see this issue.

[conseilgouz]

2 ways to fix #38310

1. update FeedFactory.php, so it does not read xml file before calling initialise function
   or
2. update RssParser & AtomParser so they assume actual xlm record is the root node

Current PR is following fix 2 idea.

[alikon]

i can replicate the issue like has been described

[conseilgouz]

i can replicate the issue like has been described

It seems that just the two of us are interested in this issue.

[fontanil]

I have tested this item ✅ successfully on 6157800

Tested with success (4.2.0 rc2 dev)

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38312.}

[conseilgouz]

@alikon : one more human test to go.....

[YGomiero]

I have tested this item ✅ successfully

Tested with success (4.2.0 rc2 dev)

[richard67]

I have tested this item ✅ successfully

Tested with success (4.2.0 rc2 dev)

@YGomiero A comment with a green check mark is not sufficient for the test to count. You have to mark your test result in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/38312 , use the blue "Test this" button at the top left corner, select the test result and finally submit. Could you do so? Thanks in advance.

[YGomiero]

I have tested this item ✅ successfully on 6157800

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38312.}

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38312.}