Unity

Merge lp://qastaging/~3v1n0/unity/testing-friend into lp://qastaging/unity

  1. testing-friend
  2. Merge into trunk
Proposed by Marco Trevisan (Treviño)
Status: Rejected
Rejected by: Andrea Azzarone
Proposed branch: lp://qastaging/~3v1n0/unity/testing-friend
Merge into: lp://qastaging/unity
Prerequisite: lp://qastaging/~3v1n0/unity/launcher-controller-ensure-new
Diff against target: 569 lines (+275/-76)
7 files modified
launcher/EdgeBarrierController.cpp (+1/-41)
launcher/EdgeBarrierController.h (+3/-3)
launcher/EdgeBarrierControllerPrivate.h (+68/-0)
launcher/LauncherController.h (+2/-2)
tests/test_edge_barrier_controller.cpp (+34/-28)
tests/test_launcher_controller.cpp (+12/-2)
unity-shared/TestingFriend.h (+155/-0)
To merge this branch: bzr merge lp://qastaging/~3v1n0/unity/testing-friend
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Disapprove
Brandon Schaefer (community) Needs Fixing
Francis Ginther Abstain
jenkins (community) continuous-integration Needs Fixing
Review via email: mp+116783@code.qastaging.launchpad.net

Commit message

TestingFriend: added testing friend, a tool that will (hopefully) help you in testing!

Description of the change

Added the unity::tests::Friend class definition and some utility macros that can be used to easily test private members of classes without great refactoring.

Basically it's only needed that the class(es) you're testing is friend of unity::tests::Friend and then on the testing side you can implement that Friend function with the proxy functions you need to access to private members.
I've added some macros that allow this definition easier. This is needed since in C++ the friendship is not transitive.

I know that the same could have been achieved just by adding a different friend class for each class that is then implemented as inheriting from ::testing::Test (as done in lp:~3v1n0/unity/launcher-controller-ensure-new for LauncherController), but I didn't like this for two reasons:
1) it would have allowed to access to all members from the tests, without any explicit control on it
2) it would have not allowed to test interactions between classes without defining friends around

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Don't you have to make sure that the unit::tests::Friend class is in an anonymous namespace in the test source file to make sure you don't get linking errors?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Don't you have to make sure that the unit::tests::Friend class is in an
> anonymous namespace in the test source file to make sure you don't get linking
> errors?

If I add it to an anonymous namespace, it won't work since it won't be considered friend of the class I've added to.

Revision history for this message
jenkins (martin-mrazik+qa) wrote :

FAILED: Continuous integration, rev:2558
http://jenkins.qa.ubuntu.com/job/unity-ci/126/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/126/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/126/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Martin Mrazik (mrazik) wrote :

Sorry for the jenkins failure. I'm looking into this.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ah, and Tim... I see no link errors here even when defining it multiple times (actually it's defined twice). Is that maybe because the definition is actually inline?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Ah, and Tim... I see no link errors here even when defining it multiple times
> (actually it's defined twice). Is that maybe because the definition is
> actually inline?

Looking at the standard, it also seems caused by the fact that these tests::Friend classes are defined in different translation units, and this will be always true (until you don't play too much with tests), so we should be and we actually are safe from linking errors.

Revision history for this message
Francis Ginther (fginther) wrote :

Review was claimed by accident, please ignore.

review: Abstain
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

In file included from /home/bschaefer/src/unity/tests/test_launcher_controller.cpp:24:0:
/home/bschaefer/src/unity/launcher/LauncherController.h: In member function ‘unity::launcher::LauncherModel::Ptr unity::launcher::TestLauncherController::GetLauncherModel()’:
/home/bschaefer/src/unity/launcher/LauncherController.h:91:25: error: ‘std::unique_ptr<unity::launcher::Controller::Impl> unity::launcher::Controller::pimpl’ is private
/home/bschaefer/src/unity/tests/test_launcher_controller.cpp:96:15: error: within this context

Also should merge with trunk as it still has the old dependency for libutouch-geis

review: Needs Fixing
Revision history for this message
Andrea Azzarone (azzar1) :
review: Disapprove

Unmerged revisions

2558. By Marco Trevisan (Treviño)

Merging with lp:~3v1n0/unity/launcher-controller-ensure-new (and trunk)

2557. By Marco Trevisan (Treviño)

test_edge_barrier_controller: use tests::Friend

2556. By Marco Trevisan (Treviño)

EdgeBarrierController: move the definition of ::Impl in EdgeBarrierControllerPrivate

For testing purposes...

2555. By Marco Trevisan (Treviño)

LauncherController: use unity::tests::Friend to access to pimpl!

2554. By Marco Trevisan (Treviño)

TestingFriend: added testing friend, a tool that will help you in testing!

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.