kucaahbe / rspec-html-matchers

Old school have_tag, with_tag(and more) matchers for rspec 3 (Nokogiri powered)
http://rubygems.org/gems/rspec-html-matchers
MIT License
199 stars 90 forks source link

Nested matchers are not necessarily applied to the same found element #41

Closed skalee closed 7 years ago

skalee commented 8 years ago

Given Rails view application/broken.html:

<div class="broken">
    <a></a>
</div>
<div class="broken">
    <span></span>
</div>
<div class="broken">
    text
</div>

Following test will pass:

require "rails_helper"

RSpec.describe "application/broken" do

  before do
    render
  end

  it "is broken" do
    rendered.should have_tag ".broken", text: /text/ do
      with_tag "span"
      with_tag "a"
    end
  end

end

As you can see, there is no element which matches all these conditions simultaneously:

  1. .broken with text matching /text/
  2. .broken span
  3. .broken a

Is it a bug or desired behaviour? If the former, I think I can fix it and send a pull request.

bryan-liff commented 7 years ago

+1 as a bug

Given:

<div>
 <foo />
</div>
<div>
 <bar />
</div>
<div>
 <foo />
 <bar />
</div>

The following 'count' fails:

expect(rendered).to have_tag('div', count: 1) do
  with_tag 'foo'
  with_tag 'bar'
end

It matches three div's when it should only match one based on the nested conditions.

Or am I doing it wrong?

randoum commented 7 years ago

@kucaahbe this PR is open since 2015 ! See also #58 related

The thing I hate the most about open-source is to see great repositories rotting because the owner has just gave up on it. Seriously, your reasons may be personal, professional, health related or what, but when you publish and maintain open-source code for people to use it you automatically have a responsibility to maintain it, simply because people are using it.

So stop ignoring email, and fix the issues. Make time for it, or find someone serious to succeed it.

kucaahbe commented 7 years ago

@randoum totally agree with you! I'm reading issues and PRs all the time, but unfortunately in most cases just have no time to address issues, hope will be able to get to this and related bugs tomorrow or kind off. My apologies to all the people having issues

randoum commented 7 years ago

@cbernaut would you mind to check #59 and see if your issues are fixed ?

bryan-liff commented 7 years ago

Sure, no problem.

On Fri, Jan 20, 2017 at 8:53 AM, randoum notifications@github.com wrote:

@cbernaut https://github.com/cbernaut would you mind to check #59 https://github.com/kucaahbe/rspec-html-matchers/pull/59 and see if your issues are fixed ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kucaahbe/rspec-html-matchers/issues/41#issuecomment-274076958, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJPG_TDNdpboNUpg0tXpbUtTcHWU2deks5rULxygaJpZM4G3vgC .

bryan-liff commented 7 years ago

Unfortunately it does not resolve my specs.

I'll try to write some specs in short order.

Thanks for the attention to this !!

randoum commented 7 years ago

Yes please. I’ll be waiting for your specs so I can try to fix the code Cheers

On Jan 20, 2017, at 19:36, cbernaut notifications@github.com wrote:

Unfortunately it does not resolve my specs.

I'll try to write some specs in short order.

Thanks for the attention to this !! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kucaahbe/rspec-html-matchers/issues/41#issuecomment-274145877, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcsyNAgMc0HvbMEWmEPgV9sX1MC0ipvks5rUP62gaJpZM4G3vgC.

randoum commented 7 years ago

@cbernaut forget about the spec, I just had a closer look at your case and I misunderstood what you was trying to do.

You are using the block to trim down what is expected to be found, rspec-html-matchers does not work like this. The first line of your spec expect(rendered).to have_tag('div', count: 1) will fail because 3 are found. You should write

expect(rendered).to have_tag 'div', count: 3
expect(rendered).to have_tag 'div' do
  with_tag 'foo'
  with_tag 'bar'
end
expect(rendered).to have_tag 'div' do
  with_tag 'foo'
  without_tag 'bar'
end
expect(rendered).to have_tag 'div' do
  without_tag 'foo'
  with_tag 'bar'
end

Bottom line, this is not a bug, it's by design

bryan-liff commented 7 years ago

OK! Thanks for the clarification - much appreciated.