mmcdole
Unit tests are failing
Expected behavior
Since v1.2.1, unit tests are broken, it seems to be the management of the tag's attribute xml:base.
Actual behavior
It seems that since you changed links management the local base isn't use if any but is it doesn't have one none are used .
Here are a very few outputs of the go test execution:
Diff:
--- Expected
+++ Actual
@@ -33,3 +33,3 @@
Type: (string) (len=4) "html",
- Value: (string) (len=62) "Example <a href=\"http://example.com/parent/test.html\">test</a>"
+ Value: (string) (len=36) "Example <a href=\"test.html\">test</a>"
}),
Test: TestParser_Parse
Messages: Feed file entry_content_xml_base_inherit_4.xml did not match expected output entry_content_xml_base_inherit_4.json
Diff:
--- Expected
+++ Actual
@@ -27,3 +27,3 @@
Email: (string) "",
- URI: (string) (len=32) "http://example.com/relative/link"
+ URI: (string) (len=14) "/relative/link"
})
Test: TestParser_Parse
Messages: Feed file entry_contributor_uri_base.xml did not match expected output entry_contributor_uri_base.json
Diff:
--- Expected
+++ Actual
@@ -18,3 +18,3 @@
Title: (string) "",
- ID: (string) (len=37) "http://example.com/test/relative/link",
+ ID: (string) (len=13) "relative/link",
Updated: (string) "",
Test: TestParser_Parse
Messages: Feed file entry_id_xml_base.xml did not match expected output entry_id_xml_base.json
Steps to reproduce the behavior
run go test ./... from the root of the project on master or from tag v1.2.1
Why are the unit tests not running as part of the pull requests? Eg - https://github.com/mmcdole/gofeed/pull/216
Screenshot
I think we broke several tests in https://github.com/mmcdole/gofeed/pull/202 .
We need to fix this. That PR was very necessary to fix concurrency issues, but it looks like we busted base/relative URL handling.
@cristoper I don't know if you'd have a hot minute to check out the broken tests + relative URL handling? Did we just miss updating some places where we parse URLs to call your new function?
Huh. I'll try to look into this soon -- feel free to ping me a reminder if you haven't heard from me next week.
Thanks for noticing these failing tests, @JLugagne. The good news is they have revealed two bugs introduced with #202 and https://github.com/mmcdole/goxpp/pull/9 that we can fix:
The main problem is that goxpp's DecodeElement() pop's the BaseStack after unmarshaling an element -- as it should to keep the BaseStack in sync with the current document scope. But that means the atom parser needs to keep a reference to the current base before calling DecodeElement() so that it can correctly resolve URLs. I've proposed one solution here: https://github.com/mmcdole/gofeed/pull/222
This also uncovered a bug in the way goxpp's DecodeElement() was maintaining the BaseStack... it would pop the stack even if the current element didn't push to it so that subsequent elements would be resolved using the wrong xml:base. I've submitted a fix for this: https://github.com/mmcdole/goxpp/pull/11
The bad news is that, except for URL attributes in the root element, gofeed has not been correctly resolving URLs according to xml:base for the past year :(
With the changes in both of the PRs above in place, go test ./... passes all tests on my machine (though we need to remember to depend on the updated version of goxpp in go.mod). The embarrassing thing is that I missed these failing tests because apparently at the time of #202 I was writing go test rather than go test ./... so only the top-level package tests were being run. The test Github Action should prevent that mistake in the future.
go test ./... passes all tests on my machine
Scratch that, I was not on the latest HEAD. There is still one RSS test that is failing since #220 (not related to the xml:base issues):
--- FAIL: TestDefaultRSSTranslator_Translate (0.14s)
translator_test.go:45:
Error Trace: gofeed/translator_test.go:45
Error: Not equal:
expected: &gofeed.Feed{Title:"", Description:"", Link:"", FeedLink:"", Links:[]string(nil), Updated:"", UpdatedParsed:<nil>, Published:"", PublishedParsed:<nil>, A
uthor:(*gofeed.Person)(nil), Authors:[]*gofeed.Person(nil), Language:"", Image:(*gofeed.Image)(nil), Copyright:"", Generator:"", Categories:[]string(nil), DublinCoreExt:(*ext.DublinCoreExtension)
(nil), ITunesExt:(*ext.ITunesFeedExtension)(nil), Extensions:ext.Extensions(nil), Custom:map[string]string(nil), Items:[]*gofeed.Item{(*gofeed.Item)(0xc0000ef8c0)}, FeedType:"rss", FeedVersion:"0
.91"}
actual : &gofeed.Feed{Title:"", Description:"", Link:"", FeedLink:"", Links:[]string(nil), Updated:"", UpdatedParsed:<nil>, Published:"", PublishedParsed:<nil>, A
uthor:(*gofeed.Person)(nil), Authors:[]*gofeed.Person(nil), Language:"", Image:(*gofeed.Image)(nil), Copyright:"", Generator:"", Categories:[]string(nil), DublinCoreExt:(*ext.DublinCoreExtension)
(nil), ITunesExt:(*ext.ITunesFeedExtension)(nil), Extensions:ext.Extensions(nil), Custom:map[string]string(nil), Items:[]*gofeed.Item{(*gofeed.Item)(0xc0000ef7a0)}, FeedType:"rss", FeedVersion:"0
.91"}
Diff:
--- Expected
+++ Actual
@@ -25,3 +25,3 @@
Description: (string) "",
- Content: (string) (len=42) "<img src=\"http://example.com/content.png\">",
+ Content: (string) "",
Link: (string) "",
@@ -35,6 +35,3 @@
GUID: (string) "",
- Image: (*gofeed.Image)({
- URL: (string) (len=30) "http://example.com/content.png",
- Title: (string) ""
- }),
+ Image: (*gofeed.Image)(<nil>),
Categories: ([]string) <nil>,
@@ -44,3 +41,5 @@
Extensions: (ext.Extensions) <nil>,
- Custom: (map[string]string) <nil>
+ Custom: (map[string]string) (len=1) {
+ (string) (len=7) "content": (string) (len=42) "<img src=\"http://example.com/content.png\">"
+ }
})
Test: TestDefaultRSSTranslator_Translate
Messages: Feed file feed_item_image_-_rss_channel_item_content.xml did not match expected output feed_item_image_-_rss_channel_item_content.json