Improve test coverage

[juanrh]

Issue #, if available:

Description of changes:
Add additional tests to improve test coverage to

Overall coverage rate:
  lines......: 93.2% (770 of 826 lines)
  functions..: 92.5% (210 of 227 functions)
  branches...: 49.4% (375 of 759 branches)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[juanrh]

Attached html coverage report
coverage.tar.gz

[AAlon]

Even though this is a ROS-agnostic package, we do depend on catkin, which provides utilities such as catkin_add_gtest, catkin_add_gmock and already takes care of setting up and building gtest/gmock if necessary (along with some fairly complex logic around path finding and edge cases: https://github.com/ros/catkin/blob/b1622e2042b8fa01df2896ea12d375acf6443beb/cmake/test/gtest.cmake#L229)

I think it'd be better to use that if possible.

[juanrh]

I have tried using catkin_add_gmock in this other branch following the documentation and the problem with this is that that target it's not able to locate the gmock installation, so colcon build logs

CMake Warning at /opt/ros/kinetic/share/catkin/cmake/test/gtest.cmake:149 (message):
  skipping gmock 'test_client_configuration_provider' in project 'aws_common'
  because gmock was not found
Call Stack (most recent call first):
  /opt/ros/kinetic/share/catkin/cmake/test/gtest.cmake:79 (_catkin_add_executable_with_google_test)
  /opt/ros/kinetic/share/catkin/cmake/test/gtest.cmake:54 (_catkin_add_google_test)
  CMakeLists.txt:116 (catkin_add_gmock)

and it skips building that test. As seen in cartographer-project/cartographer_ros#776 and ros/catkin#970 other people had the same problem. The main issue is that "CMake by default comes with FindGTest.cmake but is does not come with FindGMock.cmake", and there is no authoritative FindGMock.cmake that reliably locates the gmock install. I have found several versions of FindGMock.cmake on the internet, but all they make different assumptions about the location of the sources for gmock, and none worked out of the box with the image for kinetic.
As a result find_package(GMock) fails to locate gmock, and catkin_add_gmock skips the test. In cartographer-project/cartographer_ros#776 the requester propose using a custom one. But I don't see much value in using catkin_add_gmock if that requires writing our own FindGMock.cmake file, because we don't get the value of a reliable configuration for gmock.

Also catkin_add_gmockdoesn't generate an executable for the tests, but a make target make run_tests_aws_common_gtest_test_client_configuration_provider that, when I tested, has to build all the dependencies to the AWS SDK from scratch, instead of depending on ${aws_common_LIBRARIES} and ${PROJECT_NAME}, which makes development slower. However this is a less important issue.

So I'd prefer to keep using libgmock as originally proposed, which has worked fine for health-metrics-collector-ros1 and cloudwatchmetrics-ros1.

However, I'm open to any idea including using some FindGMock.cmake together with catkin_add_gmock, if we are able to get a reliable version.

[AAlon]

I'd be fine with the current approach, I wouldn't want this issue to block us. Overall looks good - great job adding the tests!

[AAlon]

Just a minor note, we'd want to revisit the method by which we build gmock, perhaps create an issue for it either here or internally, so we don't forget about it. But other than that looks good 👍