Landscape Client

Merge lp://qastaging/~tealeg/landscape-client/mocker-replace-test-changer into lp://qastaging/~landscape/landscape-client/trunk

  1. mocker-replace-test-changer
  2. Merge into trunk
Proposed by Geoff Teale
Status: Merged
Approved by: Geoff Teale
Approved revision: 902
Merged at revision: 905
Proposed branch: lp://qastaging/~tealeg/landscape-client/mocker-replace-test-changer
Merge into: lp://qastaging/~landscape/landscape-client/trunk
Diff against target: 460 lines (+125/-206)
1 file modified
landscape/package/tests/test_changer.py (+125/-206)
To merge this branch: bzr merge lp://qastaging/~tealeg/landscape-client/mocker-replace-test-changer
Reviewer Review Type Date Requested Status
🤖 Landscape Builder test results Approve
Simon Poirier (community) Approve
Alberto Donato (community) Approve
Review via email: mp+297681@code.qastaging.launchpad.net

Commit message

Replace mocker with mock and patch.

Note that tests that were previously writing shell script and executing those scripts have been *radically* simplified - at least one of these was previous failing to execute it's deferred assertions.

Testing instructions:

Description of the change

Replace mocker with mock and patch.

Note that tests that were previously writing shell script and executing those scripts have been *radically* simplified - at least one of these was previous failing to execute it's deferred assertions.

Testing instructions:

To post a comment you must log in.
Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: TRIAL_ARGS=-j4 make check
Result: Fail
Revno: 897
Branch: lp:~tealeg/landscape-client/mocker-replace-test-changer
Jenkins: https://ci.lscape.net/job/latch-test/5109/

review: Needs Fixing (test results)
Revision history for this message
Alberto Donato (ack) wrote :

+1 Looks good, with a few minor comments inline.

review: Approve
Revision history for this message
Simon Poirier (simpoir) wrote :

+1 looks good

review: Approve
Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 901
Branch: lp:~tealeg/landscape-client/mocker-replace-test-changer
Jenkins: https://ci.lscape.net/job/latch-test/5112/

review: Approve (test results)
Revision history for this message
Geoff Teale (tealeg) wrote :

I've addressed the comments it's viable to address.

Combining the with statements has complications that make it undesirable (I spent quite a lot of time trying to find the nicest way to do this). It could still be done in a single "with" but it would require line break escaping, and I don't think it brings us enough to justify the ugliness.

lp://qastaging/~tealeg/landscape-client/mocker-replace-test-changer updated
902. By Geoff Teale

Remove leftover

Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 902
Branch: lp:~tealeg/landscape-client/mocker-replace-test-changer
Jenkins: https://ci.lscape.net/job/latch-test/5113/

review: Approve (test results)

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: