[Merge] lp:~mardy/location-service/lp1499256 into lp:location-service

Thomas Voß thomas.voss at canonical.com
Wed Oct 14 11:45:36 UTC 2015


Review: Needs Fixing

Thanks for the changes. Two minor niggles inline.

Diff comments:

> 
> === modified file 'tests/gps_provider_test.cpp'
> --- tests/gps_provider_test.cpp	2015-01-12 08:41:14 +0000
> +++ tests/gps_provider_test.cpp	2015-10-14 11:37:11 +0000
> @@ -48,6 +48,31 @@
>  
>  namespace
>  {
> +
> +struct MockHardwareGps
> +{
> +    MockHardwareGps()
> +    {
> +        instance_ = this;
> +    }
> +    ~MockHardwareGps()
> +    {
> +        instance_ = nullptr;
> +    }
> +
> +    MOCK_METHOD5(set_position_mode, bool(uint32_t, uint32_t, uint32_t, uint32_t, uint32_t));
> +    MOCK_METHOD3(inject_time, void(int64_t, int64_t, int));
> +
> +    static MockHardwareGps *mocked(UHardwareGps self) {
> +        return reinterpret_cast<MockHardwareGps*>(self);
> +    }
> +    static MockHardwareGps *instance() { return instance_; }

A singleton is fine here, I would however prefer: static MockHardwareGps *instance() { static MockHardwareGps inst; return &inst; }
With that, you can get rid of custom ctor and dtor, as well as the static instance_.

> +
> +    static MockHardwareGps *instance_;
> +};
> +
> +MockHardwareGps *MockHardwareGps::instance_ = nullptr;
> +
>  struct UpdateTrap
>  {
>      MOCK_METHOD1(on_position_updated, void(const location::Position&));
> @@ -211,6 +236,36 @@
>  
>  }
>  
> +UHardwareGps
> +u_hardware_gps_new(UHardwareGpsParams *)

A comment in the code on the u_* functions would be helpful to understand the approach for injecting mock implementations during testing.

> +{
> +    using namespace ::testing;
> +
> +    return reinterpret_cast<UHardwareGps>(MockHardwareGps::instance());
> +}
> +
> +void
> +u_hardware_gps_delete(UHardwareGps)
> +{
> +}
> +
> +bool
> +u_hardware_gps_set_position_mode(UHardwareGps self, uint32_t mode, uint32_t recurrence,
> +                                 uint32_t min_interval, uint32_t preferred_accuracy,
> +                                 uint32_t preferred_time)
> +{
> +    MockHardwareGps *thiz = MockHardwareGps::mocked(self);
> +    return thiz->set_position_mode(mode, recurrence, min_interval,
> +                                   preferred_accuracy, preferred_time);
> +}
> +
> +void
> +u_hardware_gps_inject_time(UHardwareGps self, int64_t time, int64_t time_reference, int uncertainty)
> +{
> +    MockHardwareGps* thiz = MockHardwareGps::mocked(self);
> +    return thiz->inject_time(time, time_reference, uncertainty);
> +}
> +
>  TEST(AndroidGpsXtraDownloader, reading_configuration_from_valid_conf_file_works)
>  {
>      std::stringstream ss{gps_conf};


-- 
https://code.launchpad.net/~mardy/location-service/lp1499256/+merge/274381
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.



More information about the Ubuntu-reviews mailing list