Ok, there have been a few tickets like this (http://
dev.rubyonrails.com/ticket/2799) and I''ve been closing them
''wontfix''
because of backwards-compatibility issues. But I''m beginning to
wonder if maybe this is an instance where breaking backwards
compatibility is an acceptible cost, because the current behavior
really does feel broken.
A summary: the problem is that assert_tag uses pattern matching
(String#index) to match string values, so that:
assert_tag :tag => "div", :attributes => { :id =>
"hello" }
would match
<div id="helloworld">
This (as Eric mentions in that ticket) has the potential to cause
tests to pass when they fail, with no one the wiser.
Does anyone have any strong feelings one way or the other on this?
Any compelling reason why preserving backwards compatibility on this
is crucial? I think most people are believing strings to be exact
matches anyway, and are using them that way, so I don''t think many
people will be bitten by the change. If anything, the change might
reveal bugs that were being missed before.
- Jamis
On Nov 8, 2005, at 11:24 PM, Jamis Buck wrote:> Ok, there have been a few tickets like this (http:// > dev.rubyonrails.com/ticket/2799) and I''ve been closing them > ''wontfix'' because of backwards-compatibility issues. But I''m > beginning to wonder if maybe this is an instance where breaking > backwards compatibility is an acceptible cost, because the current > behavior really does feel broken. > > A summary: the problem is that assert_tag uses pattern matching > (String#index) to match string values, so that: > > assert_tag :tag => "div", :attributes => { :id => "hello" } > > would match > > <div id="helloworld"> > > This (as Eric mentions in that ticket) has the potential to cause > tests to pass when they fail, with no one the wiser. > > Does anyone have any strong feelings one way or the other on this? > Any compelling reason why preserving backwards compatibility on > this is crucial? I think most people are believing strings to be > exact matches anyway, and are using them that way, so I don''t think > many people will be bitten by the change. If anything, the change > might reveal bugs that were being missed before.The exception to this is that when you match the text content, you probably want a substring match. But... can anyone explain why assert_tag doesn''t just use XPath?
On Nov 8, 2005, at 9:24 PM, Jamis Buck wrote:> Does anyone have any strong feelings one way or the other on this? > Any compelling reason why preserving backwards compatibility on > this is crucial? I think most people are believing strings to be > exact matches anyway, and are using them that way, so I don''t think > many people will be bitten by the change. If anything, the change > might reveal bugs that were being missed before.Personally, I''d rather have a test, you know, actually TEST, so I''m all in favor of revealing bugs that might have been hidden before. Even at the cost of breaking compatibility. -- Deirdre Saoirse Moen deirdre@textdrive.com
On Nov 9, 2005, at 12:24 AM, Jamis Buck wrote:> Ok, there have been a few tickets like this > (http://dev.rubyonrails.com/ticket/2799) and I''ve been closing them > ''wontfix'' because of backwards-compatibility issues. But I''m beginning > to wonder if maybe this is an instance where breaking backwards > compatibility is an acceptible cost, because the current behavior > really does feel broken. > > A summary: the problem is that assert_tag uses pattern matching > (String#index) to match string values, so that: > > assert_tag :tag => "div", :attributes => { :id => "hello" } > > would match > > <div id="helloworld"> > > This (as Eric mentions in that ticket) has the potential to cause > tests to pass when they fail, with no one the wiser. > > Does anyone have any strong feelings one way or the other on this? Any > compelling reason why preserving backwards compatibility on this is > crucial? I think most people are believing strings to be exact matches > anyway, and are using them that way, so I don''t think many people will > be bitten by the change. If anything, the change might reveal bugs > that were being missed before. > > - JamisI''d vote for fixing it to match the expressed expectations. I''m pretty sure I''ve used strings in loads of places where I intend it to be an exact match, and I think that''s what makes the most sense. If it''s a well documented change and it breaks someone''s tests, at least it''s very easy for them to fix. Like you, I don''t think it will be an issue for most people. -- Scott Barron Lunchbox Software http://lunchboxsoftware.com http://lunchroom.lunchboxsoftware.com http://rubyi.st
I agree wholeheartedly with the others. I don''t think that there''s a question if people expect quotations to imply a rigid match as opposed to a substring match. If tests break after the change I expect that it will be because the tests are written under the wrong assumptions and a change like this will point out bugs in their code. To be clear, I vote to break compatability. Kevin Clark
Yup, you''d thinkg that => "string" would not be substring.
OK, what about when you want to match classname; I use css classes
heavily, so that
<h1 class="article header odd"><%= article.title
%></h1>
<h1 class="article header even"> .. </h1
So that here, you''d want a substring match for assert_tag :class=>
..
But for substring matches, I''d assume that (like EVERYTHING else it
seems in rails) you could do something like assert_tag :attributes =>
{ :class => /header/ } with a regex..
Break it!
On 11/8/05, Kevin Clark <kevin.clark@gmail.com>
wrote:> I agree wholeheartedly with the others. I don''t think that
there''s a
> question if people expect quotations to imply a rigid match as opposed
> to a substring match. If tests break after the change I expect that it
> will be because the tests are written under the wrong assumptions and
> a change like this will point out bugs in their code.
>
> To be clear, I vote to break compatability.
>
> Kevin Clark
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
In that case, would it make sense to have assert_tag look for \bSTRING \b to make sure it''s matching on a full word only? Other than that, yes, break it. -P. On Nov 9, 2005, at 8:15 AM, Courtenay wrote:> Yup, you''d thinkg that => "string" would not be substring. > > OK, what about when you want to match classname; I use css classes > heavily, so that > > <h1 class="article header odd"><%= article.title %></h1> > <h1 class="article header even"> .. </h1 > > So that here, you''d want a substring match for assert_tag :class=> .. > But for substring matches, I''d assume that (like EVERYTHING else it > seems in rails) you could do something like assert_tag :attributes => > { :class => /header/ } with a regex.. > > Break it! > > > On 11/8/05, Kevin Clark <kevin.clark@gmail.com> wrote: >> I agree wholeheartedly with the others. I don''t think that there''s a >> question if people expect quotations to imply a rigid match as >> opposed >> to a substring match. If tests break after the change I expect >> that it >> will be because the tests are written under the wrong assumptions and >> a change like this will point out bugs in their code. >> >> To be clear, I vote to break compatability. >> >> Kevin Clark >> _______________________________________________ >> Rails-core mailing list >> Rails-core@lists.rubyonrails.org >> http://lists.rubyonrails.org/mailman/listinfo/rails-core >> > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core >-- Patrick Lenz (scoop) http://poocs.net/ # Personal Weblog http://limited-overload.de/ # Web application development http://freshmeat.net/ # Free software archive http://topmedia.de/ # IT Storage Solutions
On 9-nov-2005, at 6:50, Dave Thomas wrote:> > But... can anyone explain why assert_tag doesn''t just use XPathBecause it''s supposed to work on HTML, not just XML. Manfred
On Nov 9, 2005, at 1:22 AM, Manfred Stienstra wrote:> > On 9-nov-2005, at 6:50, Dave Thomas wrote: >> >> But... can anyone explain why assert_tag doesn''t just use XPath > > Because it''s supposed to work on HTML, not just XML.Good point. I wonder though: for those who generate well formed XHTML, would XPath support be a good idea? It''s there for free in REXML, and it would be pretty powerful and concise (and would let you test stuff that''s impossible to express with the current system). Just an idle thought, really. Dave
+1 break it On 11/8/05, Courtenay <court3nay@gmail.com> wrote:> Yup, you''d thinkg that => "string" would not be substring. > > OK, what about when you want to match classname; I use css classes > heavily, so that > > <h1 class="article header odd"><%= article.title %></h1> > <h1 class="article header even"> .. </h1 > > So that here, you''d want a substring match for assert_tag :class=> .. > But for substring matches, I''d assume that (like EVERYTHING else it > seems in rails) you could do something like assert_tag :attributes => > { :class => /header/ } with a regex.. > > Break it! > > > On 11/8/05, Kevin Clark <kevin.clark@gmail.com> wrote: > > I agree wholeheartedly with the others. I don''t think that there''s a > > question if people expect quotations to imply a rigid match as opposed > > to a substring match. If tests break after the change I expect that it > > will be because the tests are written under the wrong assumptions and > > a change like this will point out bugs in their code. > > > > To be clear, I vote to break compatability. > > > > Kevin Clark > > _______________________________________________ > > Rails-core mailing list > > Rails-core@lists.rubyonrails.org > > http://lists.rubyonrails.org/mailman/listinfo/rails-core > > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core >
Just do it(tm). //jarkko On 9.11.2005, at 9.52, Obie Fernandez wrote:> +1 break it > > On 11/8/05, Courtenay <court3nay@gmail.com> wrote: >> Yup, you''d thinkg that => "string" would not be substring. >> >> OK, what about when you want to match classname; I use css classes >> heavily, so that >> >> <h1 class="article header odd"><%= article.title %></h1> >> <h1 class="article header even"> .. </h1 >> >> So that here, you''d want a substring match for assert_tag :class=> .. >> But for substring matches, I''d assume that (like EVERYTHING else it >> seems in rails) you could do something like assert_tag :attributes => >> { :class => /header/ } with a regex.. >> >> Break it! >> >> >> On 11/8/05, Kevin Clark <kevin.clark@gmail.com> wrote: >>> I agree wholeheartedly with the others. I don''t think that there''s a >>> question if people expect quotations to imply a rigid match as >>> opposed >>> to a substring match. If tests break after the change I expect >>> that it >>> will be because the tests are written under the wrong assumptions >>> and >>> a change like this will point out bugs in their code. >>> >>> To be clear, I vote to break compatability. >>> >>> Kevin Clark >>> _______________________________________________ >>> Rails-core mailing list >>> Rails-core@lists.rubyonrails.org >>> http://lists.rubyonrails.org/mailman/listinfo/rails-core >>> >> _______________________________________________ >> Rails-core mailing list >> Rails-core@lists.rubyonrails.org >> http://lists.rubyonrails.org/mailman/listinfo/rails-core >> > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core >-- Jarkko Laine http://jlaine.net http://odesign.fi -------------- next part -------------- A non-text attachment was scrubbed... Name: smime.p7s Type: application/pkcs7-signature Size: 2363 bytes Desc: not available Url : http://wrath.rubyonrails.org/pipermail/rails-core/attachments/20051109/423e8821/smime.bin
Jamis Buck wrote:> Does anyone have any strong feelings one way or the other on this? > Any compelling reason why preserving backwards compatibility on this > is crucial? I think most people are believing strings to be exact > matches anyway, and are using them that way, so I don''t think many > people will be bitten by the change. If anything, the change might > reveal bugs that were being missed before. >Please break backwards compatibility. A broken test method is a bug that must be fixed. -- stefan
I agree with the others ;)
Better to break it now than post-1.0, for the sake of people writing books if nothing else! On 11/9/05, Florian Weber <csshsh@gmail.com> wrote:> I agree with the others ;) > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core >
On Nov 9, 2005, at 12:29 AM, Dave Thomas wrote:> On Nov 9, 2005, at 1:22 AM, Manfred Stienstra wrote: > >> On 9-nov-2005, at 6:50, Dave Thomas wrote: >>> >>> But... can anyone explain why assert_tag doesn''t just use XPath >> >> Because it''s supposed to work on HTML, not just XML. > > Good point. I wonder though: for those who generate well formed > XHTML, would XPath support be a good idea? It''s there for free in > REXML, and it would be pretty powerful and concise (and would let > you test stuff that''s impossible to express with the current system). > > Just an idle thought, really.There is assert_template_xpath_match in the deprecated assertions, but it was unwieldy and fragile (and was thus deprecated). I also despise xpath. Thus, I wrote the assert_tag stuff, for making these kinds of tests more accessible. It''s a trade-off. Xpath gives you more power, but with a steeper learning curve. assert_tag has less expressive power, but is easier to learn. That''s certainly not to say assert_tag is the best approach. It can become very verbose, which I don''t like. Nathaniel Talbott sent me a patch some time ago with a proof-of-concept that built on top of assert_tag and was more concise, but I haven''t had a chance to do anything with it. And thanks all, for the feedback. I''ll fix assert_tag to use exact matches instead of substring matching. - Jamis