Code review comment for lp:~adam-collard/charms/trusty/swift-storage/add-pause-resume-actions

Hi Ryan,

Thanks for taking a look at this.

On Fri, 14 Aug 2015 at 17:10 Ryan Beisner <email address hidden>
wrote:

> RE: amulet tests
>
> Thank you for this work!
>
> Just a couple of things that will make future life easier:
>
> _run_action and _wait_on_action:
> Please consider landing these in charmhelpers/contrib/amulet/utils.py, as
> they will be increasingly-common needs.
>

Done. See charm-helpers r428

>
> _assert_services:
> Please consider enhancing the existing validate_unit_process_ids amulet
> helper to fit the needs here. I think it would be a fairly easy mod. We
> could do -x on pidof in all cases (or add a bool with default False to
> "check scripts too" if you want to be safer about it). Then add a boolean
> postive/negative switch to be able to invert its expectations (like you've
> done with should_run). fwiw, expect_success (default True) is such a bool
> name that I've used elsewhere for the same purpose in a different context
> (rmq wip).
>

I modified get_process_id_list and get_unit_process_ids but didn't end up
using validate_unit_process_ids because the validation is already done for
me in the previous two methods. See comment in the test itself.

I've split the branch up so that the new code is cleanly separated from the
fixes I made to existing code (see pre-req branch).

--
>
> https://code.launchpad.net/~adam-collard/charms/trusty/swift-storage/add-pause-resume-actions/+merge/267681
> You are the owner of
> lp:~adam-collard/charms/trusty/swift-storage/add-pause-resume-actions.
>
> Launchpad-Message-Rationale: Owner
> Launchpad-Notification-Type: code-review
> Launchpad-Branch:
> ~adam-collard/charms/trusty/swift-storage/add-pause-resume-actions
>