Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider deprecating assert_select with a numeric arg #58

Open
chancancode opened this issue Feb 15, 2017 · 6 comments
Open

Consider deprecating assert_select with a numeric arg #58

chancancode opened this issue Feb 15, 2017 · 6 comments

Comments

@chancancode
Copy link
Member

Given a page like so:

<h1>Users</h1>

<table>
  ...
</table>

Total: <span class="total">4781</span>

It's pretty reasonable to write something like this:

get "/users"
assert_select "h1", "Users"
assert_select ".total", User.count

However, that would give you a very confusing error:

Failure:
MyApp::UsersTest#test_index:
Expected exactly 4781 elements matching ".total", found 1..
Expected: 4781
  Actual: 1

Given that we already support assert_select ".total", count: User.count, I think we should consider deprecating the numeric positional argument overload (as shown in the example) in favor of the explicit keyword argument, and potentially reclaiming the space for automatic to_s-ing the positional argument in the future (to make the example "just work").

@rafaelfranca
Copy link
Member

👍

@chancancode
Copy link
Member Author

@rafaelfranca can we still add the deprecation to 5.1?

@rafaelfranca
Copy link
Member

Yes, we can. Even more that this would be a deprecation in a gem, so the deprecation would also happen in 5.0. I'm fine with it.

@chancancode
Copy link
Member Author

@rafaelfranca we can (probably should) bump the gem version, then we can decide whether we need to lock it in Rails 5.0

@rafaelfranca
Copy link
Member

yes, a minor version should be sufficient

@kaspth
Copy link
Contributor

kaspth commented Mar 9, 2017

Yes please!

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

No branches or pull requests

3 participants