thoughtworks / pacto

Pacto settles disputes between JSON providers and consumers
thoughtworks.github.io/pacto
MIT License
401 stars 58 forks source link

Fix nil issue of example_uri_values(contract) #160

Open ruanwz opened 9 years ago

ruanwz commented 9 years ago

I got following error when simulate the contract with url like: http://abc.com?a=3&d=4

 Failure/Error: result = contracts.simulate_consumers
 TypeError:
   no implicit conversion of nil into Hash
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/actors/from_examples.rb:35:in `merge!'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/actors/from_examples.rb:35:in `build_request'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/consumer.rb:70:in `build_request'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/consumer.rb:52:in `reenact'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/contract.rb:64:in `execute'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/contract.rb:40:in `simulate_request'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/contract_set.rb:9:in `map'
 # /home/david/.rvm/gems/ruby-2.1.1/gems/pacto-0.4.0.rc1/lib/pacto/contract_set.rb:9:in `simulate_consumers'
coveralls commented 9 years ago

Coverage Status

Coverage remained the same when pulling c1f3e7b02b40521f4b90f669ca110a287832b683 on ruanwz:master into 399ab8142b4382ab4d2a01e7f5d798acfde8fdfe on thoughtworks:master.

maxlinc commented 9 years ago

Thanks @ruanwz. Could you send a sample contract or create a test scenario? I think you found a bug, but maybe not quite the one you thought.

That line obviously can return nil, but I don't think it should so setting a default value might not be the best fix. I suspect the actual problem is related to optional slashes in the path. The URI template is probably built with a slash, but then getting called with a URI that doesn't contain one:

t = Addressable::Template.new('http://abc.com/{?othervars*}')

# works with a slash
uri = Addressable::URI.heuristic_parse('http://abc.com/?a=3&d=4')
t.extract uri
# => {"othervars"=>{"a"=>"3", "d"=>"4"}}

# fails without a slash
uri = Addressable::URI.heuristic_parse('http://abc.com?a=3&d=4')
t.extract uri
# => nil

Many APIs don't care if the final slash is present or not, so Pacto should probably treat '/?a=3&d=4' and '?a=3&d=4' (or '/foo?a=3&d=4' vs '/foo/?a=3&d=4') as equivalent by default, but with an option to be more strict about slashes.