have_selector and have_xpath don't match descendants in blocks
Reported by tomo | April 29th, 2009 @ 02:52 AM
HaveXpath (and thus HaveSelector) replace %r'//' with './' in the xpath query within a block. This prevents matchers inside the block from matching parents of the matched nodes. It also prevents matchers inside the block from matching arbitrary descendants of the matched nodes. See http://gist.github.com/103587 for an example.
Also, if the query xpath happened to have more than one '//' in it (e.g. '//foo//bar'), wouldn't this be severely screwed up ('./foo./bar')?
I think this can be solved by instead replacing %r'^//' with './/'. I've forked webrat and added a couple tests: http://github.com/tomo/webrat. Making this change didn't break any tests, but I'm not very confident that it doesn't introduce any bugs.
Comments and changes to this ticket
-
tomo April 29th, 2009 @ 10:13 PM
- Tag set to “have_selector, have_xpath, patch”
changing this to a patch: http://github.com/tomo/webrat
-
tomo April 29th, 2009 @ 10:50 PM
just realized this is a duplicate of https://webrat.lighthouseapp.com... and is fixed at http://github.com/cavalle/webrat...
I think the the gsub should also be changed to only replace an initial //, because otherwise //foo//bar will get replaced with .//foo.//bar, when it should be just .//foo//bar (right?)
-
gaffo May 10th, 2009 @ 10:26 PM
- Tag changed from “have_selector, have_xpath, patch” to “have_selector, have_xpath, patch, stale”
- State changed from “new” to “open”
tomo, Thanks for the patch and the report. I'm a bit confused by your comments above. Should I just close this one as a duplicate in favor of #220?
-
tomo May 11th, 2009 @ 02:27 AM
cavalle's patch mostly fixes it, but a small problem remains. For example, see http://gist.github.com/109883.
>The descendant selector inside the block converts to xpath like
//...//..., and both currenthave_xpathand cavalle's fix replace the second//and cause an Nokogiri::XML::XPath::SyntaxError.The regex in the gsub just needs a
^at the beginning. We should only replace the first//with.//. -
Jason Garber May 13th, 2009 @ 02:29 PM
Tomo's seems to be the better patch. Please merge it in ASAP. Wasted a couple hours today trying to work around this bug.
-
gaffo May 13th, 2009 @ 03:16 PM
- Tag changed from “have_selector, have_xpath, patch, stale” to “have_selector, have_xpath, patch, verify”
- State changed from “open” to “awaiting-patch”
So to clarify, please put in the patch that works. I'm taking it that this is not now related to #220?
-
tomo May 13th, 2009 @ 03:41 PM
Here's the patch I did originally: http://github.com/tomo/webrat/commit/9a3668be92927084a2cce2ea3888f2...
It's pretty related to #220... same fix except for an extra ^.
-
tomo May 13th, 2009 @ 03:57 PM
I just added another spec to make sure have_xpath works with descendant selectors inside the block: http://github.com/tomo/webrat/commit/232ed36379104484100f3aab100487...
Though actually, I accidentally revealed that bug in the spec I wrote before, because the gsub currently reaches into the href attribute of the link selector and changes the url.
-
Bryan Helmkamp June 2nd, 2009 @ 07:45 PM
- State changed from “awaiting-patch” to “resolved”
(from [28c676b4fcb1004d2c8de48fe75ef34e7a9dd63e]) [#234 state:resolved] added attribution http://github.com/brynary/webrat/commit/28c676b4fcb1004d2c8de48fe75...
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
Ruby Acceptance Testing for Web applications.
gaffo
tomo
Referenced by
-
234
have_selector and have_xpath don't match descendants in blocks
(from [28c676b4fcb1004d2c8de48fe75ef34e7a9dd63e])
[#234
s...