Thanks for this. I had a number of suggestions. Also if those .moved
files are in the repo that needs to be taken care of. After that is done
can you re-propose and I can approve it
In general for functional tests with spin up times I prefer to assert as
much as possible in a single test. As long as you don't depend on state
manipulation beyond the initial setup that is fine.
Thanks for this. I had a number of suggestions. Also if those .moved
files are in the repo that needs to be taken care of. After that is done
can you re-propose and I can approve it
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/00_ setup.sh. moved setup.sh. moved (right):
File tests/00_
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/00_ setup.sh. moved#newcode3 setup.sh. moved:3: sudo apt-get install install amulet
tests/00_
python-requests
This file appears to be an artifact of some bzr op and should be removed
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/100_ deploy_ cloud.test deploy_ cloud.test (right):
File tests/100_
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/100_ deploy_ cloud.test# newcode33 deploy_ cloud.test: 33: search_term = ;".format( mysql_unit. info['public- address' ])
tests/100_
"\"{}\"
Minor but its usually better to use single quotes for the string rather
than escape as the result is more readable
'"{}";'.format(...)
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/10_ cache_relation_ test.py cache_relation_ test.py (right):
File tests/10_
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/10_ cache_relation_ test.py# newcode16 cache_relation_ test.py: 16: # Add mediawiki and mysql db server with_memcache seem redundant. Given that
tests/10_
This file and the 100_deploy_
theses tests are not free we should remove one.
In general for functional tests with spin up times I prefer to assert as
much as possible in a single test. As long as you don't depend on state
manipulation beyond the initial setup that is fine.
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/10_ db_relation_ test.py db_relation_ test.py (right):
File tests/10_
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/10_ db_relation_ test.py# newcode32 db_relation_ test.py: 32:
tests/10_
Again this test is very similar to the deploy_cloud test. Is there a
reason to have twice as much code at twice the runtime cost? Can we
combine theses?
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/lib/ helper. py
File tests/lib/helper.py (right):
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/lib/ helper. py#newcode7 helper. py:7: 'Accept': application/ xhtml+xml, application/ xml;q=0. 9,*/*',
tests/lib/
'text/html,
as a style point defining this above the method as
LOGIN_HEADERS = {
...
}
can save you some indentation space and make things a little easier to
read.
https:/ /codereview. appspot. com/54600043/ diff/1/ tests/lib/ helper. py#newcode15 helper. py:15: def standup_ unit(deployment , seconds=900):
tests/lib/
deploy as a name? This could bring up more than one unit, no?
https:/ /codereview. appspot. com/54600043/