Could a core member take a moment to commit these patches?

I've got a couple small patches that are quite helpful and minimally
invasive that I'd love to get out of my vendor/rails so I can start
work on new patches with a clean edge rails (without checking out
another copy). If any core member has time to take a look at these
and either commit these or give me some feedback I'd really appreciate
it:

http://dev.rubyonrails.org/ticket/8683

Koz looked at this one and asked for a test 3 weeks ago, which I
provided, but I haven't heard back. This one is particularly useful
for development because without it, Template Missing errors don't
actually say what template was requested (broken since the addition of
multiple controller view paths).

http://dev.rubyonrails.org/ticket/8173

This one's been out there for several months with nary a comment
(other than "not so tiny", which I dispute). It makes XML testing
with assert_select work. The only condition under which it would
break someone's existing tests would be if they were sending content
type application/xhtml+xml (which no one does because IE doesn't
understand it) AND they were producing invalid XHTML (an unlikely
combination to say the least). So it seems like a no-brainer to me.
Jamis even blogged about a workaround
(http://weblog.jamisbuck.org/2007/1/4/assert_xml_select), so it's
surprising there's been no comment.

Thanks in advance,
Gabe da Silveira

I've got a couple small patches that are quite helpful and minimally
invasive that I'd love to get out of my vendor/rails so I can start
work on new patches with a clean edge rails (without checking out
another copy). If any core member has time to take a look at these
and either commit these or give me some feedback I'd really appreciate
it:

http://dev.rubyonrails.org/ticket/8683

Koz looked at this one and asked for a test 3 weeks ago, which I
provided, but I haven't heard back. This one is particularly useful
for development because without it, Template Missing errors don't
actually say what template was requested (broken since the addition of
multiple controller view paths).

Applied. Thanks!

http://dev.rubyonrails.org/ticket/8173

This one's been out there for several months with nary a comment
(other than "not so tiny", which I dispute). It makes XML testing
with assert_select work. The only condition under which it would
break someone's existing tests would be if they were sending content
type application/xhtml+xml (which no one does because IE doesn't
understand it) AND they were producing invalid XHTML (an unlikely
combination to say the least). So it seems like a no-brainer to me.
Jamis even blogged about a workaround
(http://weblog.jamisbuck.org/2007/1/4/assert_xml_select), so it's
surprising there's been no comment.

assert_select and xml is way outside my comfort zone so I'll pass on
this one. Anyone else have any comments on it?

If you can get three regular contributors to report on the utility of
the patch, and the implementation, I'd feel more comfortable applying
it.

Applied. Thanks!

Thank you!

> http://dev.rubyonrails.org/ticket/8173

assert_select and xml is way outside my comfort zone so I'll pass on
this one. Anyone else have any comments on it?

If you can get three regular contributors to report on the utility of
the patch, and the implementation, I'd feel more comfortable applying
it.

I know there's gotta be people out there bitten by this one. If your
XML schema contains auto-closing HTML tags (such as <area>) then
testing with assert_select becomes unusable. The patch is so simple,
if you're doing RESTful APIs and like the power of assert_select,
please take 5 minutes to check it out. It'll save you a lot of ugly
hacks.

I've been bitten by this before, and would love to see assert_select
made more friendly for xml.

- Rob

How are css selectors useful when testing xml? Do you just use the
element names?

As for the patch, you should probably use the lookup function in
mime.rb instead of regexp matching. That'll remove the false positives

Gabe da Silveira wrote:

I know there's gotta be people out there bitten by this one.

Yes. The case I got bitten by is the <link> element: in HTML it's self-closing, in feed XML it is not.

I currently use the method from http://weblog.jamisbuck.org/2007/1/4/assert_xml_select, which defines a separate assert method for the XML case.

It looks like the original assert_select plug-in may have had something similar in the form of assert_select_feed (it still features in Edge comments, but the code doesn't seem to have survived.)

I don't know of any reason not to fix up the underlying response as in your patch, though. Anyone? I do think the method name seems wrong now - it's not really an html_document you're returning. Should the fix really be applied in response_from_page_or_rjs?

I note that the patch fixes css_select too, as well as assert_select - it currently suffers from the same set of XML problems.

  --Anthony.

How are css selectors useful when testing xml? Do you just use the
element names?

element names, attributes, child, ancestor, sibling, pseudo-classes
like :empty, etc. Pretty much anything but .class and #id selectors
are widely useful. Think about testing your XML with regexps (yech!)
or installing a parser. assert_select provides an amazingly powerful
syntax for a non-trivial problem domain. If only web browsers
supported all those selectors!

As for the patch, you should probably use the lookup function in
mime.rb instead of regexp matching. That'll remove the false positives

Agreed, this will be an improvement. I'm checking out mime.rb now to
figure out how to do this.

Okay, I've gone in and made the patch using the lookup function.
However, after looking closer at the situation, I believe the original
patch is better for a number of reasons which I've detailed in the
patch comments.

I don't believe there is any possibilty of a false positive, because
the ONLY type that should be parsed as HTML is text/html. Everything
else (including XHTML) should be parsed as XML. There are numerous
types of XML, including user-defined XML formats, all of which use a
standard mime_type convention of application/type+xml, so there is no
way to get around the use of a regexp. Meanwhile, the lookup function
requires Rails-known mime types, even though the developer can specify
any content-type he wants.

So far it seems a few people are interested in the patch. I haven't
marked it verified, is the rule for that you have to get the explicit
three +1s? Should I go on rails-contrib? I would think the mailing
list gets more eyes, but I'm ready to do whatever it takes to get this
puppy committed.