Stringify param values in controller tests. #1203
Conversation
This reduces false positives that come from using ints in params in tests, which do not get converted to strings in the tests. In implementations going through rack, they do get converted to strings. - David Chelimsky and Sam Umbach
|
Wouldn't it be better to call to_param? To save rehashing old arguments look at these tickets: https://rails.lighthouseapp.com/projects/8994/tickets/2984 |
|
to_param would definitely be more appropriate as we would be able to pass models to get/post and friends. @dchelimsky, what do you think? |
|
to_param makes sense (I hadn't seen those tickets, sry). That said, I'm a bit confused. https://rails.lighthouseapp.com/projects/8994/tickets/2984 says it's resolved, but I don't see a patch. What happened? Also - we need to exclude Rack::Test::UploadedFile from this as the to_param conversion does the wrong thing. Are there any other exclusions you can think of? |
- exclude Rack::Test::UploadedFile to pass existing tests. Are there any other types we're missing?
|
+1 for to_paramification. |
…n-tests Stringify param values in controller tests.
|
@josevalim - I see this is in master but not 3-1-stable. Is the intent to include this post 3.1? |
|
@dchelimsky we can include it on 3-1. Could you please send a pull request for 3-1-stable (I am without my mac lately)? |
|
@josevalim - I squashed the two commits into a single commit and cherry-picked it onto 3-1-stable in a separate pull request: #2253 |
|
Per https://twitter.com/#!/nzkoz/status/177832158617018368, this breaks the ability to functional test json APIs (which support fixnums). Any thoughts on how to preserve the benefits from this change and also support json APIs? The first thing that comes to my mind is a declaration in the test like |
|
Well, this change breaks backwards compatibility and should only be applied when it's definitely safe, but that's me ;) I think the simplest option is to only do this to_paramization when the format of the request is :html or nil, that at least lets people include :format=>:json and hopefully move on with their lives? |
|
I'd suggest discussing in that issue rather than here as this one is closed and ancient |
This reduces false positives that come from using ints in params in
tests, which do not get converted to strings in the tests. In
implementations going through rack, they do get converted to strings.