Merge lp://staging/~jkakar/landscape-client/grr-mocking into lp://staging/~landscape/landscape-client/trunk

Proposed by Jamu Kakar
Status: Merged
Approved by: Jamu Kakar
Approved revision: 228
Merged at revision: not available
Proposed branch: lp://staging/~jkakar/landscape-client/grr-mocking
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 96 lines (+61/-3)
2 files modified
landscape/manager/eucalyptus.py (+2/-2)
landscape/manager/tests/test_eucalyptus.py (+59/-1)
To merge this branch: bzr merge lp://staging/~jkakar/landscape-client/grr-mocking
Reviewer Review Type Date Requested Status
Free Ekanayaka (community) Approve
Muharem Hrnjadovic (community) Approve
Review via email: mp+22929@code.staging.launchpad.net

Description of the change

This branch fixes the function signatures for start_service_hub and
get_eucalyptus_info. The first 'self' parameter was not removed
when they were refactored from methods to functions, and because of
the use of mocking they were not tested directly and so no tests
actually broke. I've fixed the signatures and added (mocking-based,
ugh) tests to ensure they can be called as expected.

To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Took me a while to understand what's going :) Looks good!

review: Approve
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Nice fix! +1

[1]

+ self.expect(
+ euca_service_factory(reactor,
+ "/data/path/eucalyptus")).result(euca_service)

In case the expectation to declare doesn't fit in a single line, I find it more readable to write it like:

+ euca_service_factory(reactor, "/data/path/eucalyptus")
+ self.mocker.result(euca_service)

which also saves one line in this case. It's just a matter of taste though.

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

to all changes: