collectiveidea / json_spec

Easily handle JSON in RSpec and Cucumber
rubygems.org/gems/json_spec
MIT License
919 stars 114 forks source link

Add probability testing matcher #56

Closed taiki45 closed 10 years ago

taiki45 commented 10 years ago

Thanks to this great gem!:gem:

When I use this to my game app API server's cucumber testing, I wanted to test the API response which have probability. And worse than that, the app can't fix probability in testing.

So I wanted to write cucumber feature like this:

Then the JSON at "1/id" should be one of 1, 2

I wrote a step to test that in my app test like this:

Then /^(?:JSON|json)(?: response)?(?: at "(.*)")? should( not)? be one of (.*)$/ do |path, negative, exp|
  if negative
    eval(exp).to_json.should_not include_json(JsonSpec.remember(find_value_by_path(last_json, path)))
  else
    eval(exp).to_json.should include_json(JsonSpec.remember(find_value_by_path(last_json, path)))
  end
end

def find_value_by_path(json, path)
  JsonSpec::Helpers.normalize_json(json, path)
end

In feature file:

And JSON response at "list/1/id" should be one of [1, 2]

And I thought this is useful!

So I suggest be_one_of matcher to this great gem!!

be_one_of RSpec matcher is almost ornament because everyone can use include? or something to test probability behavior. But in Cucumber, we want a handy step. So I suggested this matcher.

How do you think?

charlierudolph commented 10 years ago

I'm not a big fan of this. I don't know of any other test frameworks that support something like this. To me you are testing two different things with this matcher. For your json test, I think you should stub out the probability (i know you said you can't, but I don't think this is an appropriate solution. Open to have my mind changed but thats thought for now.

taiki45 commented 10 years ago

@charlierudolph hmm, I think you are right. And this was very dirty way...

I worked for legacy application so I wanted something like this. But normally, something like this is NOT acceptable.

I think this issue and pull-request should be closed (without merge). Thank you for reviewing!