Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] Fix tests for Travis-CI build #570

Merged
merged 1 commit into from Jan 31, 2014
Merged

Conversation

@redapple
Copy link
Contributor

@redapple redapple commented Jan 30, 2014

scrapy.tests.test_contrib_exporter.XmlItemExporterTest still need work

@redapple redapple mentioned this pull request Jan 30, 2014
@redapple
Copy link
Contributor Author

@redapple redapple commented Jan 30, 2014

Finally passing!

Link(url='http://www.google.com/something', text=''),
])
self.assertEqual(
set([link for link in lx.extract_links(self.response)]),

This comment has been minimized.

@kmike

kmike Jan 30, 2014
Member

is comparing sets still necessary? I.e. is the order of links link extractor produces semi-random?

This comment has been minimized.

@redapple

redapple Jan 30, 2014
Author Contributor

should not. let's see...

@@ -5,6 +5,7 @@

from scrapy.link import Link
from .sgml import SgmlLinkExtractor
from scrapy.utils.python import unique

This comment has been minimized.

@kmike

kmike Jan 30, 2014
Member

is this import used?

This comment has been minimized.

@redapple

redapple Jan 30, 2014
Author Contributor

not anymore no

This comment has been minimized.

@kmike

kmike Jan 30, 2014
Member

and that's good :) it fixes a bug where unique=False had no effect on RegexLinkExtractor


class XmlItemExporterTest(BaseItemExporterTest):

def _get_exporter(self, **kwargs):
return XmlItemExporter(self.output, **kwargs)

def assertXmlEqual(self, first, second, msg=None):

This comment has been minimized.

@kmike

kmike Jan 30, 2014
Member

the solution I came up with some time ago: http://stackoverflow.com/a/7060342/114795

This comment has been minimized.

@redapple

redapple Jan 30, 2014
Author Contributor

hey, that's nice :)

This comment has been minimized.

@redapple

redapple Jan 30, 2014
Author Contributor

sadly, this didn't work (or I missed something)
[https://travis-ci.org/scrapy/scrapy/builds/17937034]

This comment has been minimized.

@kmike

kmike Jan 30, 2014
Member

Yep - the XML is different. Attribute order in XML doesn't matter, but element order does; it seems that our XmlItemExported exports fields in some random order, and tests show that. So this assert shouldn't be named assertXmlEqual because it checks that two xml snippets with all children sorted are equal, not that two xml snippets are equal.

This comment has been minimized.

@redapple

redapple Jan 30, 2014
Author Contributor

assertXmlEquivalent?

This comment has been minimized.

@kmike

kmike Jan 31, 2014
Member

Yes, this name looks fine. Another option is to sort fields by key on export, and it has some advantages (easier testing, maybe better diffs), but that's out of the scope here.

dangra added a commit that referenced this pull request Jan 31, 2014
[MRG] Fix tests for Travis-CI build
@dangra dangra merged commit 1e553ec into scrapy:master Jan 31, 2014
1 check passed
1 check passed
default The Travis CI build passed
Details
@dangra
Copy link
Member

@dangra dangra commented Jan 31, 2014

this work unblocked the queue of PRs.
thanks @redapple and @kmike!

@redapple redapple deleted the redapple:travistests branch Jan 31, 2014
@redapple
Copy link
Contributor Author

@redapple redapple commented Jan 31, 2014

you're welcome @dangra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.