savonrb / wasabi

A simple WSDL parser
MIT License
90 stars 84 forks source link

Fix issue when searching for 1.2 SOAP operations #26

Closed andrewhr closed 11 years ago

andrewhr commented 11 years ago

This code was raising exceptions about not having a variable declared in case of 1.2 operations.

I've wanted to add at least an integration test for this fix, but didn't find any WSDL to break the actual code. And unfortunately, the WSDL used to discover the issue was a proprietary service from a current client, so I'm not allowed to share :cry:

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling dface3fd7fd3ae2c005fa2e8d93391c7cd3edc73 on andrewhr:master into 6e3ad7ebe0dcf9d62288cab7b4ec0972506b3d81 on savonrb:master.

rubiii commented 11 years ago

thanks @andrewhr. the code is obviously wrong. not sure why it wasn't covered with tests, since the fixtures actually include an example for a soap 1.2 service, but if you look at the changelog, the next release will probably be a major release containing the rewrite. so that piece of code is already gone.

andrewhr commented 11 years ago

Great! But the library rewrite is not available on current master? Because is from the master that I catch this error.

I've been using Savon for a while and the gem is awesome! Thank you for the work, you rock! :tada: :confetti_ball:

Can I help you with something to release this improvements and fixes? I will love to rely on the WSDL for the services I need to consume (actually, the current release is not able to handle then).

rubiii commented 11 years ago

thanks :blush:

the new code is on wasabi master. it's not integrated with savon yet. there's still a lot of things to do (see #27), but i'll let you know when this can be tested through savon.

andrewhr commented 11 years ago

Ok! I'll visit the issues later and see if I can pick something. (at least make the build green on jruby!)

rubiii commented 11 years ago

@andrewhr that would be awesome. i started investigating it yesterday but wasn't able to reduce the testcase to something i could show to the nokogiri guys. it looks like a difference between the c- and java-versions, but for some reason it worked fine in a simple testcase.

rubiii commented 11 years ago

@andrewhr i was looking at the wrong element. i opened a bug at sparklemotion/nokogiri#902.

andrewhr commented 11 years ago

I was investing yesterday but didn't write an example to show to nokogiri's team (in fact, I never did something like that before).

So I just add a comment about my investigation to your open issue. Hope it helps!

rubiii commented 11 years ago

@andrewhr great. thank you very much!

rubiii commented 11 years ago

@andrewhr if you think you can replace the wsdl with a more simple example, please feel free to open a pull request. it probably helps debugging the problem.

andrewhr commented 11 years ago

@rubiii I will do it now! :ok_hand: