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

Savon tests break because of the #46 fix #53

Open
jeroenj opened this issue Feb 2, 2015 · 2 comments
Open

Savon tests break because of the #46 fix #53

jeroenj opened this issue Feb 2, 2015 · 2 comments

Comments

@jeroenj
Copy link
Contributor

jeroenj commented Feb 2, 2015

If you would use the vies wsdl used in the Savon tests and would compare the parsed wsdl from 3.3.0 with 3.3.1 you would see that there are some differences. Because of that the Savon tests are failing.

My knowledge of SOAP is not good enough to determine what exactly is wrong. It feels like the Savon tests have been wrong from before this change but I'm not 100% sure.

It's also a bit confusing that this change was released with 3.3.1 while it wasn't mentioned in the changelog. I'd make a pull request for it but I have no idea how I can summarize this change in a single line.

Let me know if there is any way I can help out.

If this issue is fixed the wasabi dependency in savon can be loosened to ~> 3.3.2 (not lower since 3.3.1 is not supported because of this issue).

This would also close the following savon issues:

@tjarratt
Copy link
Contributor

tjarratt commented Mar 2, 2015

Thanks for opening this @jeroenj. This is a bit of a difficult story to tell. I'm not even sure that I could summarize the entire story myself, although I was one of the principal actors.

Basically, @davidsantoso and I tried to improve (or fix) some edge cases in how Wasabi determines the operation name. Doing so seemed to break some existing cases in Savon, but I became convinced that they had always been wrong, and reverted the tests. In doing so, I released a version of Savon that actually broke a lot of users.

I've reverted the changes related to pull request #46 here in Wasabi, and have bumped the dependency in Savon for now. Hoping to release a new version of those gems later today after all of the tests go green.

The only open question is "How can we implement the changes in #46 without breaking the world". It should be possible, but I'm unsure how.

@jeroenj
Copy link
Contributor Author

jeroenj commented Mar 3, 2015

Thanks for the info and the update. It indeed sounds like a hard problem to solve. I'll have a look at it too. :)

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

2 participants