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

Add Assertion.prototype.xmleql #661

Merged
merged 1 commit into from
May 6, 2013
Merged

Add Assertion.prototype.xmleql #661

merged 1 commit into from
May 6, 2013

Conversation

bartvde
Copy link
Member

@bartvde bartvde commented Apr 25, 2013

This is based on the xml_eq function we had in the OL2 test framework but
adapted to expect.js.

@twpayne
Copy link
Contributor

twpayne commented Apr 25, 2013

LGTM

@tschaub
Copy link
Member

tschaub commented Apr 25, 2013

Thanks for this work @bartvde. Are the error messages sufficient to figure out where the failure comes from? I'm in favor of merging regardless, but curious what the output on failure looks like.

@elemoine
Copy link
Member

elemoine commented May 2, 2013

Please merge.

This is based on the xml_eq function we had in the OL2 test framework but
adapted to expect.js.
@bartvde
Copy link
Member Author

bartvde commented May 6, 2013

@tschaub I'll merge this first and then look into your question, and possibly come up with a new PR. Thanks for pointing this out.

@elemoine
Copy link
Member

elemoine commented May 6, 2013

@tschaub I'll merge this first and then look into your question, and possibly come up with a new PR. Thanks for pointing this out.

Thanks Bart. That's what we said during last week's dev meeting.

bartvde pushed a commit that referenced this pull request May 6, 2013
@bartvde bartvde merged commit 10b690b into openlayers:master May 6, 2013
@bartvde
Copy link
Member Author

bartvde commented May 6, 2013

there were some serious flaws here on further inspection (did not really test anything since it was not using documentElement), so I'll definitely update with a new PR

@bartvde
Copy link
Member Author

bartvde commented May 6, 2013

see #689

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

Successfully merging this pull request may close these issues.

None yet

4 participants