Tests for XmlRpcValue and XML conversion.

[trainman419]

Unit tests to cover existing functionality.
Moved the logging class definition to the XmlRpcUtil header since the corresponding functions are implemented in XmlRpcUtil.cpp. This makes it easier to break XmlRpcpp into sub-libraries for future unit tests.

I've also update the include path in xmlrpcvalue_base64.cpp because our internal build system does not like the relative include path (and it's more consistent with the rest of the library).

[mikepurvis]

The tests look reasonable to me, and there should be no ill-effect from moving the header contents, since either one you include will still see the same or a superset of the symbols from before.

👍

[dirk-thomas]

Since xmlrpcpp is an external library which has been pulled into this repo I would rather prefer to avoid any changes to it (if not necessary to fix bugs). Would it be possible to do the same testing without moving the code?

[trainman419]

The more complex mocking that I'm doing for the other tests relies on having the library broken into smaller pieces so that I can mock the individual pieces without depending on all of the headers.

The biggest change here is moving the logging classes out of XmlRpc.h and into XmlRpcUtil.h (since they're implemented in XmlRpcUtil.cpp anyway), so that individual cpp files that use logging don't need to depend on all of the other headers.

[dirk-thomas]

Would it make sense to import the tests from the upstream project? Which additional parts are you trying to cover with the test suite?

Since XmlRpcUtil.h is a transitive header of XmlRpc.h the move will be transparent to other code using it. I am just trying to avoid diverging / modifying third-party code if possible. If that is the only change to the third-party code and is absolutely necessary for your testing I am fine with it.

[mikepurvis]

Looking at the git history, it seems like there was almost no change from late 2011 until a handful of restructurings earlier this year around poll/select and switching out the base64 library. Meanwhile, the upstream project hasn't released anything since 2003: http://xmlrpcpp.sourceforge.net/

Basically, I would argue that there will never be another "import" of upstream development for this package, so we should feel at liberty to iterate it forward in a way that suits the needs of its users in the ROS ecosystem and particularly roscpp.

[trainman419]

This appears to be the upstream project: https://sourceforge.net/projects/xmlrpcpp/files/ and it hasn't been updated since 2003 and doesn't have any unit tests. It looks like ROS has moved the upstream tests into the standalone-tests directory because they can't be run in an automated way.

I tried to cover most of the library with the test suite that I've written. The tests in this PR are the simple ones that cover some of the existing functionality and confirm that it works correctly.

[dirk-thomas]

It looks like ROS has moved the upstream tests into the standalone-tests directory because they can't be run in an automated way.

I was mostly wondering if it makes sense to write tests from scratch or rather iterate on the existing tests and make them work.

Anyway I am fine merging this. Thank you for working on the additional test coverage.

[trainman419]

@dirk-thomas yes, these tests were originally derived from the upstream TestValues.cpp and TestXml.cpp tests, but they've been heavily modified to run within gtest and extended to give more complete test coverage.