Code review comment for lp:~bcsaller/charm-tools/test-cfg

This is an interesting concept of defining order and making it explicit.
I didn't quite grasp it until I saw the unit tests for it. I'm not
opposed to it now, test.py:652 is still a concern but my comment on 473
now no longer applies

https://codereview.appspot.com/49670043/diff/1/charmtools/test.py
File charmtools/test.py (right):

https://codereview.appspot.com/49670043/diff/1/charmtools/test.py#newcode473
charmtools/test.py:473: 'order should be defined using only include and
skip')
Isn't it counter intuitive to have both include and skip? By design if
you only have an `include` key it'll skip everything not in that include
key. Likewise, if you have a skip it's understood that you'll do
everything but. Seems this should just be OR

https://codereview.appspot.com/49670043/diff/1/charmtools/test.py#newcode652
charmtools/test.py:652: rules = parse_substrates(cfg)
This wouldn't work as cfg is a TestCfg object that only has everything
in the 'options' key. Substrate data would be in 'substrates' at the
same level of 'options'. So if parse_substrates took the test_plan file
and did a parsing that would work. Alternatively TestCfg could (should?)
parse what's in substrates and store it as part of the object so you can
just pass cfg as you're doing here instead of re-parsing the
test_config.yaml file

https://codereview.appspot.com/49670043/