Merge lp://qastaging/~compiz-team/compiz/compiz.tests_1042537.3 into lp://qastaging/compiz/0.9.8

Proposed by Sam Spilsbury
Status: Merged
Approved by: Daniel van Vugt
Approved revision: 3375
Merged at revision: 3387
Proposed branch: lp://qastaging/~compiz-team/compiz/compiz.tests_1042537.3
Merge into: lp://qastaging/compiz/0.9.8
Prerequisite: lp://qastaging/~compiz-team/compiz/compiz.revert_3353
Diff against target: 4180 lines (+2643/-618)
31 files modified
compizconfig/gsettings/gsettings_backend_shared/gsettings_util.c (+1/-1)
compizconfig/gsettings/tests/test_gsettings_conformance.cpp (+3/-1)
compizconfig/libcompizconfig/include/ccs-defs.h (+3/-0)
compizconfig/libcompizconfig/include/ccs.h (+32/-25)
compizconfig/libcompizconfig/src/CMakeLists.txt (+16/-0)
compizconfig/libcompizconfig/src/ccs-private.h (+0/-3)
compizconfig/libcompizconfig/src/ccs_settings_upgrade_internal.c (+241/-10)
compizconfig/libcompizconfig/src/ccs_settings_upgrade_internal.h (+12/-0)
compizconfig/libcompizconfig/src/ccs_text_file.c (+212/-0)
compizconfig/libcompizconfig/src/ccs_text_file.h (+49/-0)
compizconfig/libcompizconfig/src/ccs_text_file_interface.c (+50/-0)
compizconfig/libcompizconfig/src/ccs_text_file_interface.h (+67/-0)
compizconfig/libcompizconfig/src/main.c (+131/-265)
compizconfig/libcompizconfig/tests/CMakeLists.txt (+20/-1)
compizconfig/libcompizconfig/tests/compizconfig_test_ccs_mock_backend_conformance.cpp (+21/-18)
compizconfig/libcompizconfig/tests/compizconfig_test_ccs_settings_upgrade_internal.cpp (+725/-12)
compizconfig/libcompizconfig/tests/compizconfig_test_ccs_text_file.cpp (+49/-0)
compizconfig/mocks/libcompizconfig/CMakeLists.txt (+11/-0)
compizconfig/mocks/libcompizconfig/compizconfig_ccs_context_mock.h (+3/-3)
compizconfig/mocks/libcompizconfig/compizconfig_ccs_plugin_mock.h (+15/-15)
compizconfig/mocks/libcompizconfig/compizconfig_ccs_setting_mock.h (+18/-18)
compizconfig/mocks/libcompizconfig/compizconfig_ccs_text_file_mock.cpp (+104/-0)
compizconfig/mocks/libcompizconfig/compizconfig_ccs_text_file_mock.h (+84/-0)
compizconfig/tests/compizconfig_backend_concept_test.h (+49/-246)
compizconfig/tests/compizconfig_ccs_item_in_list_matcher.h (+106/-0)
compizconfig/tests/compizconfig_ccs_list_equality.h (+74/-0)
compizconfig/tests/compizconfig_ccs_list_wrapper.h (+199/-0)
compizconfig/tests/compizconfig_ccs_list_wrapper.h.moved (+199/-0)
compizconfig/tests/compizconfig_ccs_mocked_allocator.h (+22/-0)
compizconfig/tests/compizconfig_ccs_setting_value_operators.h (+105/-0)
compizconfig/tests/compizconfig_test_value_combiners.h (+22/-0)
To merge this branch: bzr merge lp://qastaging/~compiz-team/compiz/compiz.tests_1042537.3
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Tim Penhey (community) Approve
jenkins (community) continuous-integration Approve
Review via email: mp+125993@code.qastaging.launchpad.net

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

Commit message

Put some tests in place for the upgrade code, and optimized the upgrade code in the meantime. Fixes bug 1042537

Description of the change

Put some tests in place for the upgrade code, and optimized the upgrade code in the meantime (some parts of the upgrade code were also totally broken too. Fixed them).

cleanups:
 1) constified part of the interface
 2) fix copyright headers
 3) split out text file read/write code into its own class

This branch was accidentally merged for some reason. I've reverted it and re-proposed it here.

To post a comment you must log in.
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Tim Penhey (thumper) wrote : Posted in a previous version of this proposal

My first throught is why oh why does

compiz::config::impl::CCSSettingValueListWrapper start with CCS? This prefix is surely no longer needed.

Seems weird to me that you would have a magic length number instead of just calling strlen
  static const char *upgrade = "upgrade";
  static const unsigned int upgrade_str_len = 7; // why not initialize to strlen(upgrade)?

Oh, have I mentioned how much I hate C?

You don't need to include both gtest and gmock. <gmock/gmock.h> includes gtest.h.

None of these comments are blockers to merging.

review: Approve
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> My first throught is why oh why does
>
> compiz::config::impl::CCSSettingValueListWrapper start with CCS? This prefix
> is surely no longer needed.

You are correct. Shall I remove these here or in a follow up MP based on this one?

>
> Seems weird to me that you would have a magic length number instead of just
> calling strlen
> static const char *upgrade = "upgrade";
> static const unsigned int upgrade_str_len = 7; // why not initialize to
> strlen(upgrade)?

Could also work I guess.

>
> Oh, have I mentioned how much I hate C?

Careful. :)

>
> You don't need to include both gtest and gmock. <gmock/gmock.h> includes
> gtest.h.
>
> None of these comments are blockers to merging.

Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

As far as I can tell, this is not required for quantal. So please repropose it after we've branched for 0.9.9.

review: Needs Resubmitting
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

As discussed, this is required for Q, so resubmitted to 0.9.8.4

Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tim Penhey (thumper) wrote :

Given the need to get the gsettings stuff under test nicely for Q, I feel we need this.

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

I can't break it at least. Even under valgrind.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches