From f223d5a98db5fc1505ffcd04941093856ba0b3fb Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 15 May 2017 14:54:40 +0200 Subject: Fix bug that chose the wrong header icon if a connected device has a charge but its charging/discharging state is unknown. (LP: #1470080). --- tests/test-device.cc | 376 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 268 insertions(+), 108 deletions(-) (limited to 'tests') diff --git a/tests/test-device.cc b/tests/test-device.cc index 7e25e81..12f89eb 100644 --- a/tests/test-device.cc +++ b/tests/test-device.cc @@ -1,27 +1,32 @@ /* -Copyright 2012 Canonical Ltd. + * Copyright 2012-2016 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ -Authors: - Charles Kerr - -This program is free software: you can redistribute it and/or modify it -under the terms of the GNU General Public License version 3, as published -by the Free Software Foundation. - -This program is distributed in the hope that it will be useful, but -WITHOUT ANY WARRANTY; without even the implied warranties of -MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR -PURPOSE. See the GNU General Public License for more details. - -You should have received a copy of the GNU General Public License along -with this program. If not, see . -*/ +#include "device.h" +#include "service.h" #include #include + +#include #include // ceil() -#include "device.h" -#include "service.h" +#include + class DeviceTest : public ::testing::Test { @@ -337,7 +342,11 @@ TEST_F(DeviceTest, IconNames) g_string_append_printf (expected, "%s-100-charging;", kind_str); g_string_append_printf (expected, "gpm-%s-100-charging;", kind_str); g_string_append_printf (expected, "%s-full-charging-symbolic;", kind_str); - g_string_append_printf (expected, "%s-full-charging", kind_str); + g_string_append_printf (expected, "%s-full-charging;", kind_str); + g_string_append_printf (expected, "%s-100;", kind_str); + g_string_append_printf (expected, "gpm-%s-100;", kind_str); + g_string_append_printf (expected, "%s-full-symbolic;", kind_str); + g_string_append_printf (expected, "%s-full", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); g_string_truncate (expected, 0); @@ -351,7 +360,13 @@ TEST_F(DeviceTest, IconNames) g_string_append_printf (expected, "%s-080-charging;", kind_str); g_string_append_printf (expected, "gpm-%s-080-charging;", kind_str); g_string_append_printf (expected, "%s-full-charging-symbolic;", kind_str); - g_string_append_printf (expected, "%s-full-charging", kind_str); + g_string_append_printf (expected, "%s-full-charging;", kind_str); + g_string_append_printf (expected, "%s-090;", kind_str); + g_string_append_printf (expected, "gpm-%s-090;", kind_str); + g_string_append_printf (expected, "%s-080;", kind_str); + g_string_append_printf (expected, "gpm-%s-080;", kind_str); + g_string_append_printf (expected, "%s-full-symbolic;", kind_str); + g_string_append_printf (expected, "%s-full", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); g_string_truncate (expected, 0); @@ -365,7 +380,13 @@ TEST_F(DeviceTest, IconNames) g_string_append_printf (expected, "%s-060-charging;", kind_str); g_string_append_printf (expected, "gpm-%s-060-charging;", kind_str); g_string_append_printf (expected, "%s-good-charging-symbolic;", kind_str); - g_string_append_printf (expected, "%s-good-charging", kind_str); + g_string_append_printf (expected, "%s-good-charging;", kind_str); + g_string_append_printf (expected, "%s-050;", kind_str); + g_string_append_printf (expected, "gpm-%s-050;", kind_str); + g_string_append_printf (expected, "%s-060;", kind_str); + g_string_append_printf (expected, "gpm-%s-060;", kind_str); + g_string_append_printf (expected, "%s-good-symbolic;", kind_str); + g_string_append_printf (expected, "%s-good", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); g_string_truncate (expected, 0); @@ -379,7 +400,13 @@ TEST_F(DeviceTest, IconNames) g_string_append_printf (expected, "%s-040-charging;", kind_str); g_string_append_printf (expected, "gpm-%s-040-charging;", kind_str); g_string_append_printf (expected, "%s-low-charging-symbolic;", kind_str); - g_string_append_printf (expected, "%s-low-charging", kind_str); + g_string_append_printf (expected, "%s-low-charging;", kind_str); + g_string_append_printf (expected, "%s-030;", kind_str); + g_string_append_printf (expected, "gpm-%s-030;", kind_str); + g_string_append_printf (expected, "%s-040;", kind_str); + g_string_append_printf (expected, "gpm-%s-040;", kind_str); + g_string_append_printf (expected, "%s-low-symbolic;", kind_str); + g_string_append_printf (expected, "%s-low", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); g_string_truncate (expected, 0); @@ -393,7 +420,13 @@ TEST_F(DeviceTest, IconNames) g_string_append_printf (expected, "%s-000-charging;", kind_str); g_string_append_printf (expected, "gpm-%s-000-charging;", kind_str); g_string_append_printf (expected, "%s-caution-charging-symbolic;", kind_str); - g_string_append_printf (expected, "%s-caution-charging", kind_str); + g_string_append_printf (expected, "%s-caution-charging;", kind_str); + g_string_append_printf (expected, "%s-010;", kind_str); + g_string_append_printf (expected, "gpm-%s-010;", kind_str); + g_string_append_printf (expected, "%s-000;", kind_str); + g_string_append_printf (expected, "gpm-%s-000;", kind_str); + g_string_append_printf (expected, "%s-caution-symbolic;", kind_str); + g_string_append_printf (expected, "%s-caution", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); g_string_truncate (expected, 0); @@ -496,15 +529,13 @@ TEST_F(DeviceTest, IconNames) g_string_append_printf (expected, "%s-caution-symbolic;", kind_str); g_string_append_printf (expected, "%s-caution", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); - g_string_truncate (expected, 0); - // state unknown - g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind, - INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_UNKNOWN, + // if we know the charge level, but not that it’s charging, + // then we should use the same icons as when it’s discharging. + // https://wiki.ubuntu.com/Power?action=diff&rev2=78&rev1=77 + // https://bugs.launchpad.net/ubuntu/+source/indicator-power/+bug/1470080 + g_object_set (o, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_UNKNOWN, NULL); - g_string_append_printf (expected, "%s-missing-symbolic;", kind_str); - g_string_append_printf (expected, "gpm-%s-missing;", kind_str); - g_string_append_printf (expected, "%s-missing", kind_str); EXPECT_ICON_NAMES_EQ(expected->str, device); g_string_truncate (expected, 0); } @@ -704,6 +735,101 @@ TEST_F(DeviceTest, Inestimable___this_takes_80_seconds) g_free (real_lang); } +namespace +{ + const std::array,UP_DEVICE_KIND_LAST> kinds = { + std::make_pair("unknown", UP_DEVICE_KIND_UNKNOWN), + std::make_pair("line-power", UP_DEVICE_KIND_LINE_POWER), + std::make_pair("battery", UP_DEVICE_KIND_BATTERY), + std::make_pair("ups", UP_DEVICE_KIND_UPS), + std::make_pair("monitor", UP_DEVICE_KIND_MONITOR), + std::make_pair("mouse", UP_DEVICE_KIND_MOUSE), + std::make_pair("keyboard", UP_DEVICE_KIND_KEYBOARD), + std::make_pair("pda", UP_DEVICE_KIND_PDA), + std::make_pair("phone", UP_DEVICE_KIND_PHONE), + std::make_pair("media-player", UP_DEVICE_KIND_MEDIA_PLAYER), + std::make_pair("tablet", UP_DEVICE_KIND_TABLET), + std::make_pair("computer", UP_DEVICE_KIND_COMPUTER) + }; + const std::string& kind2str(UpDeviceKind kind) + { + return std::find_if( + kinds.begin(), + kinds.end(), + [kind](decltype(kinds[0])& i){return i.second==kind;} + )->first; + } + UpDeviceKind str2kind(const std::string& str) + { + return std::find_if( + kinds.begin(), + kinds.end(), + [str](decltype(kinds[0])& i){return i.first==str;} + )->second; + } + + /** + **/ + + const std::array,UP_DEVICE_STATE_LAST> states = + { + std::make_pair("unknown", UP_DEVICE_STATE_UNKNOWN), + std::make_pair("charging", UP_DEVICE_STATE_CHARGING), + std::make_pair("discharging", UP_DEVICE_STATE_DISCHARGING), + std::make_pair("empty", UP_DEVICE_STATE_EMPTY), + std::make_pair("charged", UP_DEVICE_STATE_FULLY_CHARGED), + std::make_pair("pending-charge", UP_DEVICE_STATE_PENDING_CHARGE), + std::make_pair("pending-discharge", UP_DEVICE_STATE_PENDING_DISCHARGE) + }; + const std::string& state2str(UpDeviceState state) + { + return std::find_if( + states.begin(), + states.end(), + [state](decltype(states[0])& i){return i.second==state;} + )->first; + } + UpDeviceState str2state(const std::string& str) + { + return std::find_if( + states.begin(), + states.end(), + [str](decltype(states[0])& i){return i.first==str;} + )->second; + } + + /** + **/ + + std::string device2str(IndicatorPowerDevice* device) + { + std::ostringstream o; + const auto path = indicator_power_device_get_object_path(device); + + o << kind2str(indicator_power_device_get_kind(device)) + << ' ' << state2str(indicator_power_device_get_state(device)) + << ' ' << indicator_power_device_get_time(device)<<'m' + << ' ' << int(ceil(indicator_power_device_get_percentage(device)))<<'%' + << ' ' << (path ? path : "nopath"); + + return o.str(); + } + + IndicatorPowerDevice* str2device(const std::string& str) + { + auto tokens = g_strsplit(str.c_str(), " ", 0); + g_assert(5u == g_strv_length(tokens)); + const auto kind = str2kind(tokens[0]); + const auto state = str2state(tokens[1]); + const time_t time = atoi(tokens[2]); + const double pct = strtod(tokens[3],nullptr); + const char* path = !g_strcmp0(tokens[4],"nopath") ? nullptr : tokens[4]; + auto ret = indicator_power_device_new(path, kind, pct, state, time); + g_strfreev(tokens); + return ret; + } +} + /* If a device has multiple batteries and uses only one of them at a time, they should be presented as separate items inside the battery menu, but everywhere else they should be aggregated (bug 880881). @@ -714,96 +840,130 @@ TEST_F(DeviceTest, Inestimable___this_takes_80_seconds) should be the the maximum of the times for all those that are charging. */ TEST_F(DeviceTest, ChoosePrimary) { - struct Description - { - const char * path; - UpDeviceKind kind; - UpDeviceState state; - guint64 time; - double percentage; - }; - - const Description descriptions[] = { - { "/some/path/d0", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 10, 60.0 }, // 0 - { "/some/path/d1", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 20, 80.0 }, // 1 - { "/some/path/d2", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 30, 100.0 }, // 2 - - { "/some/path/c0", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 10, 60.0 }, // 3 - { "/some/path/c1", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 20, 80.0 }, // 4 - { "/some/path/c2", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 30, 100.0 }, // 5 - - { "/some/path/f0", UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_FULLY_CHARGED, 0, 100.0 }, // 6 - { "/some/path/m0", UP_DEVICE_KIND_MOUSE, UP_DEVICE_STATE_DISCHARGING, 20, 80.0 }, // 7 - { "/some/path/m1", UP_DEVICE_KIND_MOUSE, UP_DEVICE_STATE_FULLY_CHARGED, 0, 100.0 }, // 8 - { "/some/path/pw", UP_DEVICE_KIND_LINE_POWER, UP_DEVICE_STATE_UNKNOWN, 0, 0.0 } // 9 - }; - - std::vector devices; - for(const auto& desc : descriptions) - devices.push_back(indicator_power_device_new(desc.path, desc.kind, desc.percentage, desc.state, time_t(desc.time))); - const struct { - std::vector device_indices; - Description expected; + std::string description; + std::string expected; + std::vector devices; } tests[] = { - - { { 0 }, descriptions[0] }, // 1 discharging - { { 0, 1 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 20, 70.0 } }, // 2 discharging - { { 1, 2 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 30, 90.0 } }, // 2 discharging - { { 0, 1, 2 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 30, 80.0 } }, // 3 discharging - - { { 3 }, descriptions[3] }, // 1 charging - { { 3, 4 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 20, 70.0 } }, // 2 charging - { { 4, 5 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 30, 90.0 } }, // 2 charging - { { 3, 4, 5 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 30, 80.0 } }, // 3 charging - - { { 6 }, descriptions[6] }, // 1 charged - { { 6, 0 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 10, 80.0 } }, // 1 charged, 1 discharging - { { 6, 3 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_CHARGING, 10, 80.0 } }, // 1 charged, 1 charging - { { 6, 0, 3 }, { nullptr, UP_DEVICE_KIND_BATTERY, UP_DEVICE_STATE_DISCHARGING, 10, 73.3 } }, // 1 charged, 1 charging, 1 discharging - - { { 0, 7 }, descriptions[0] }, // 1 discharging battery, 1 discharging mouse. pick the one with the least time left. - { { 2, 7 }, descriptions[7] }, // 1 discharging battery, 1 discharging mouse. pick the one with the least time left. - - { { 0, 8 }, descriptions[0] }, // 1 discharging battery, 1 fully-charged mouse. pick the one that's discharging. - { { 6, 7 }, descriptions[7] }, // 1 discharging mouse, 1 fully-charged battery. pick the one that's discharging. - - { { 0, 9 }, descriptions[0] }, // everything comes before power lines - { { 3, 9 }, descriptions[3] }, - { { 7, 9 }, descriptions[7] } + { + "one discharging battery", + "battery discharging 10m 60% bat01", + { "battery discharging 10m 60% bat01" } + }, + { + "merge two discharging batteries", + "battery discharging 20m 70% nopath", + { "battery discharging 10m 60% bat01", "battery discharging 20m 80% bat02" } + }, + { + "merge two other discharging batteries", + "battery discharging 30m 90% nopath", + { "battery discharging 20m 80% bat01", "battery discharging 30m 100% bat02" } + }, + { + "merge three discharging batteries", + "battery discharging 30m 80% nopath", + { "battery discharging 10m 60% bat01", "battery discharging 20m 80% bat02", "battery discharging 30m 100% bat03" } + }, + { + "one charging battery", + "battery charging 10m 60% bat01", + { "battery charging 10m 60% bat01" } + }, + { + "merge two charging batteries", + "battery charging 20m 70% nopath", + { "battery charging 10m 60% bat01", "battery charging 20m 80% bat02" } + }, + { + "merge two other charging batteries", + "battery charging 30m 90% nopath", + { "battery charging 20m 80% bat01", "battery charging 30m 100% bat02" } + }, + { + "merge three charging batteries", + "battery charging 30m 80% nopath", + { "battery charging 10m 60% bat01", "battery charging 20m 80% bat02", "battery charging 30m 100% bat03" } + }, + { + "one charged battery", + "battery charged 0m 100% bat01", + { "battery charged 0m 100% bat01" } + }, + { + "merge one charged, one discharging", + "battery discharging 10m 80% nopath", + { "battery charged 0m 100% bat01", "battery discharging 10m 60% bat02" } + }, + { + "merged one charged, one charging", + "battery charging 10m 80% nopath", + { "battery charged 0m 100% bat01", "battery charging 10m 60% bat02" } + }, + { + "merged one charged, one charging, one discharging", + "battery discharging 10m 74% nopath", + { "battery charged 0m 100% bat01", "battery charging 10m 60% bat02", "battery discharging 10m 60% bat03" } + }, + { + "one discharging mouse and one discharging battery. pick the one with the least time left", + "battery discharging 10m 60% bat01", + { "battery discharging 10m 60% bat01", "mouse discharging 20m 80% mouse01" } + }, + { + "one discharging mouse and a different discharging battery. pick the one with the least time left", + "mouse discharging 20m 80% mouse01", + { "battery discharging 30m 100% bat01", "mouse discharging 20m 80% mouse01" } + }, + { + "everything comes before power lines #1", + "battery discharging 10m 60% bat01", + { "battery discharging 10m 60% bat01", "line-power unknown 0m 0% lp01" } + }, + { + "everything comes before power lines #2", + "battery charging 10m 60% bat01", + { "battery charging 10m 60% bat01", "line-power unknown 0m 0% lp01" } + }, + { + "everything comes before power lines #2", + "mouse discharging 20m 80% mouse01", + { "mouse discharging 20m 80% mouse01", "line-power unknown 0m 0% lp01" } + }, + { + // https://bugs.launchpad.net/ubuntu/+source/indicator-power/+bug/1470080/comments/10 + "don't select a device with unknown state when we have another device with a known state...", + "battery charged 0m 100% bat01", + { "battery charged 0m 100% bat01", "phone unknown 0m 61% phone01" } + }, + { + // https://bugs.launchpad.net/ubuntu/+source/indicator-power/+bug/1470080/comments/10 + "...but do select the unknown state device if nothing else is available", + "phone unknown 0m 61% phone01", + { "phone unknown 0m 61% phone01" } + } }; for(const auto& test : tests) { - const auto& x = test.expected; - - GList * device_glist = nullptr; - for(const auto& i : test.device_indices) - device_glist = g_list_append(device_glist, devices[i]); + // build the device list + GList* device_glist {}; + for (const auto& description : test.devices) + device_glist = g_list_append(device_glist, str2device(description)); + // run the test auto primary = indicator_power_service_choose_primary_device(device_glist); - EXPECT_STREQ(x.path, indicator_power_device_get_object_path(primary)); - EXPECT_EQ(x.kind, indicator_power_device_get_kind(primary)); - EXPECT_EQ(x.state, indicator_power_device_get_state(primary)); - EXPECT_EQ(x.time, indicator_power_device_get_time(primary)); - EXPECT_EQ(int(ceil(x.percentage)), int(ceil(indicator_power_device_get_percentage(primary)))); - g_object_unref(primary); + EXPECT_EQ(test.expected, device2str(primary)); + g_clear_object(&primary); // reverse the list and repeat the test // to confirm that list order doesn't matter - device_glist = g_list_reverse (device_glist); - primary = indicator_power_service_choose_primary_device (device_glist); - EXPECT_STREQ(x.path, indicator_power_device_get_object_path(primary)); - EXPECT_EQ(x.kind, indicator_power_device_get_kind(primary)); - EXPECT_EQ(x.state, indicator_power_device_get_state(primary)); - EXPECT_EQ(x.time, indicator_power_device_get_time(primary)); - EXPECT_EQ(int(ceil(x.percentage)), int(ceil(indicator_power_device_get_percentage(primary)))); - g_object_unref(primary); + device_glist = g_list_reverse(device_glist); + primary = indicator_power_service_choose_primary_device(device_glist); + EXPECT_EQ(test.expected, device2str(primary)); + g_clear_object(&primary); // cleanup - g_list_free(device_glist); + g_list_free_full(device_glist, g_object_unref); } - - for (auto& device : devices) - g_object_unref (device); } -- cgit v1.2.3