Merge lp://staging/~chris.macnaughton/charm-helpers/pids-can-be-a-list into lp://staging/charm-helpers

1. See inline comments. That existing validation block is already particularly non-beautiful ;-) and the commentary helps clarify. Let's keep that consistent in that block.

2. The variable name e_pids_length (expected process IDs length) makes sense for the existing validation blocks, but it is confusing/mis-named in the context of it being a list type. Ideas to address that?

3. I'd like to see this validated before merge by syncing this c-h LP branch into ceph-mon, then updating basic_deployment.py like so:

<icey> a ceph-mon test
<icey> looking...
<icey> beisner: https://review.openstack.org/#/c/287446/7/tests/basic_deployment.py
<icey> line 185, changed from {'ceph-osd': 2} to {'ceph-osd': True}
<icey> would rather be: {'ceph-osd': [2, 3]}

4. And finally by running the test, capturing and commenting here with a pastebin of the test
output.

4. (amulet) test that is.

5. Please also add a simple 1-liner docstring to each new unit test case so that when nosetests are run verbosely, we see that output.

Ha! Never mind 5. You've already done that. I just can't read apparently.

Do you still want to see the changes against ceph-osd Ryan, or are the test cases enough to make you comfortable?

Thanks for the unit test coverage on this. That shows that the behavior is as intended. If we are to omit a sync/amulet test, we should also add negative unit tests here, where you pass bad scenarios and assert that the validation method has returned something other than None.

Can you add 1-per? That would make this rock solid.

I'm seeing test_accepts_bool_wrong & test_accepts_string_wrong fail in this rev (fresh venv, via `make test`).

Please update the *_wrong docstrings to reflect the negative aspect.

Also, I believe the *_wrong assertions should be IsNotNone.

Thanks for your work on this. Looks good!