Skip to content

Conversation

kaspth
Copy link
Contributor

@kaspth kaspth commented Nov 18, 2014

Fixes #26.

I was wondering if we should run an always passing assertion though.

Imagine a user writing assert_select 'body', 0 and then running the tests to find the assertion count doesn't go up.

What do you think, @rafaelfranca and @chancancode?

@kaspth
Copy link
Contributor Author

kaspth commented Nov 18, 2014

Only the rubinius version failed. Travis says the gems might be corrupted. Do you know of a fix, Rafael?

@rafaelfranca
Copy link
Member

I didn't get the always passing assertion. Could you explain better?

@kaspth
Copy link
Contributor Author

kaspth commented Nov 18, 2014

I couldn't find an assertion to run in this case except one that will always pass like assert true.

Kasper

Den 18/11/2014 kl. 15.51 skrev Rafael Mendonça França notifications@github.com:

I didn't get the always passing assertion. Could you explain better?


Reply to this email directly or view it on GitHub.

@rafaelfranca
Copy link
Member

I see. I think we should run them.

About rbx, let remove it from our suite from now.

We need to change code on Rails right?

@kaspth
Copy link
Contributor Author

kaspth commented Nov 18, 2014

Okay, I'll add an assert true.

Yes, we need to revert this: rails/rails#17107

@kaspth
Copy link
Contributor Author

kaspth commented Nov 18, 2014

@rafaelfranca I've changed it a bit now. No longer is assert_size_match polluted and a more elaborate comment is added.

Note I'm calling return after running the assert true so this will break:

assert_select 'body', 0 do
  # block won't be run
end

Is that okay? I can't think of a situation where people would nest a call to a document that had no body.

Rafael Mendonça França and others added 3 commits November 18, 2014 20:44
We're creating Nokogiri documents from HTML fragments, which means Nokogiri automatically
adds a body element which is unexpected.
@rafaelfranca
Copy link
Member

I don't think this block case is valid. If you are using 0 as count you don't want to see the element in the response so it doesn't make sense to assert something inside it.

rafaelfranca added a commit that referenced this pull request Nov 18, 2014
Fix #26 by running assertion when we aren't testing for a body.
@rafaelfranca rafaelfranca merged commit 0500ae5 into master Nov 18, 2014
@rafaelfranca rafaelfranca deleted the special-body branch November 18, 2014 20:38
@rafaelfranca
Copy link
Member

Applied rails/rails@1b9e85d

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

Successfully merging this pull request may close these issues.

2 participants