Merge lp:~compiz-team/compiz/compiz.fix_1042041 into lp:compiz/0.9.8

Proposed by Sam Spilsbury
Status: Merged
Approved by: Martin Mrazik
Approved revision: 3397
Merged at revision: 3390
Proposed branch: lp:~compiz-team/compiz/compiz.fix_1042041
Merge into: lp:compiz/0.9.8
Prerequisite: lp:~compiz-team/compiz/compiz.fix_1041535.1
Diff against target: 110 lines (+54/-9)
2 files modified
compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting.c (+1/-1)
compizconfig/integration/gnome/gsettings/tests/compizconfig_test_ccs_gnome_gsettings_integrated_setting.cpp (+53/-8)
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_1042041
Reviewer Review Type Date Requested Status
jenkins (community) continuous-integration Approve
Daniel van Vugt Approve
Review via email: mp+125992@code.launchpad.net

This proposal supersedes a proposal from 2012-09-24.

Commit message

Actually change string settings to the new value, not the current one
(LP: #1042041)

Description of the change

Actually change string settings to the new value, not the current one. Tests updated (with matcher to actually check what value is being set, not that a value is being set):
CCSGSettingsIntegratedSettingTestMismatchedValues/CCSGSettingsIntegratedSettingTest.MatchedTypesReturnValueMismatchedTypesResetOrWrite/6 (0 ms)[ OK ]
CCSGSettingsIntegratedSettingTestMismatchedValues/CCSGSettingsIntegratedSettingTest.MatchedTypesReturnValueMismatchedTypesResetOrWrite/15 (0 ms)[ OK ]

To post a comment you must log in.
Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Blocked, waiting for fixes to the prerequisite branch.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Looks fine (though I think the very long lines limit readability).

Tests pass.

Valgrind is happy too.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/288/console reported an error when processing this lp:~compiz-team/compiz/compiz.fix_1042041 branch.
Not merging it.

Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Revision history for this message
Martin Mrazik (mrazik) wrote :

I'm not sure why the autolanding failed but it might be related to low disk space. I'm re-approving to see if it happens again.

Revision history for this message
Unity Merger (unity-merger) wrote :

The prerequisite lp:~compiz-team/compiz/compiz.fix_1041535.1 has not yet been merged into lp:compiz.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting.c'
2--- compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting.c 2012-09-25 08:43:23 +0000
3+++ compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting.c 2012-09-25 08:43:23 +0000
4@@ -192,7 +192,7 @@
5 if (currentValue)
6 {
7 if (strcmp (currentValue, newValue) != 0)
8- writeStringToVariant (currentValue, &newVariant);
9+ writeStringToVariant (newValue, &newVariant);
10 }
11 }
12 }
13
14=== modified file 'compizconfig/integration/gnome/gsettings/tests/compizconfig_test_ccs_gnome_gsettings_integrated_setting.cpp'
15--- compizconfig/integration/gnome/gsettings/tests/compizconfig_test_ccs_gnome_gsettings_integrated_setting.cpp 2012-09-25 08:43:23 +0000
16+++ compizconfig/integration/gnome/gsettings/tests/compizconfig_test_ccs_gnome_gsettings_integrated_setting.cpp 2012-09-25 08:43:23 +0000
17@@ -69,6 +69,7 @@
18 GVariant * s ();
19 GVariant * b ();
20 GVariant * as ();
21+ GVariant * fromValue (CCSSettingValue *v, CCSSettingType type);
22 }
23
24 namespace value_generators
25@@ -124,6 +125,11 @@
26 }
27 }
28
29+MATCHER_P (VariantEqual, lhs, "Variants Equal")
30+{
31+ return g_variant_equal (lhs, arg);
32+}
33+
34 namespace
35 {
36 std::map <CCSSettingType, SpecialOptionType> &
37@@ -169,6 +175,41 @@
38 };
39
40 GVariant *
41+ccvg::fromValue (CCSSettingValue *v,
42+ CCSSettingType type)
43+{
44+ switch (type)
45+ {
46+ case TypeInt:
47+ return g_variant_new ("i", v->value.asInt);
48+ break;
49+ case TypeBool:
50+ return g_variant_new ("b", v->value.asBool);
51+ break;
52+ case TypeString:
53+ return g_variant_new ("s", v->value.asString);
54+ break;
55+ case TypeKey:
56+ {
57+ GVariantBuilder builder;
58+ g_variant_builder_init (&builder, G_VARIANT_TYPE ("as"));
59+
60+ /* Represented internally as strings */
61+ std::string kb (v->value.asString);
62+ if (kb == "Disabled")
63+ kb[0] = 'd';
64+
65+ g_variant_builder_add (&builder, "s", kb.c_str ());
66+ return g_variant_builder_end (&builder);
67+ }
68+ default:
69+ break;
70+ }
71+
72+ return NULL;
73+}
74+
75+GVariant *
76 ccvg::as ()
77 {
78 GVariantBuilder builder;
79@@ -368,19 +409,23 @@
80 mWrapper.get (),
81 &ccsDefaultObjectAllocator);
82
83- CCSSettingValue *value = (*integratedSettingInfo.valueGenerator) ();
84- GVariant *variant = (*integratedSettingInfo.variantGenerator) ();
85- EXPECT_CALL (*mWrapperMock, getValue (Eq (keyName))).WillOnce (Return (variant));
86+ boost::shared_ptr <CCSSettingValue> value ((*integratedSettingInfo.valueGenerator) (),
87+ boost::bind (ccsFreeSettingValueWithType,
88+ _1,
89+ integratedSettingInfo.settingType));
90+ boost::shared_ptr <GVariant> variant = AutoDestroy (g_variant_ref ((*integratedSettingInfo.variantGenerator) ()),
91+ g_variant_unref);
92+ boost::shared_ptr <GVariant> newVariant = AutoDestroy (ccvg::fromValue (value.get (),
93+ integratedSettingInfo.settingType),
94+ g_variant_unref);
95+ EXPECT_CALL (*mWrapperMock, getValue (Eq (keyName))).WillOnce (Return (variant.get ()));
96
97 if (createSettingType == integratedSettingInfo.settingType)
98- EXPECT_CALL (*mWrapperMock, setValue (Eq (keyName), _)); // can't verify this right yet
99+ EXPECT_CALL (*mWrapperMock, setValue (Eq (keyName), VariantEqual (newVariant.get ())));
100 else
101 EXPECT_CALL (*mWrapperMock, resetKey (Eq (keyName)));
102
103- ccsIntegratedSettingWriteValue (gsettingsIntegrated, value, createSettingType);
104-
105- if (value)
106- ccsFreeSettingValueWithType (value, integratedSettingInfo.settingType);
107+ ccsIntegratedSettingWriteValue (gsettingsIntegrated, value.get (), createSettingType);
108 }
109
110 INSTANTIATE_TEST_CASE_P (CCSGSettingsIntegratedSettingTestMismatchedValues, CCSGSettingsIntegratedSettingTest,

Subscribers

People subscribed via source and target branches