-

Adds integration tests for unit deploy, db relationship with mysql, and cache relationship with memcached

https://code.launchpad.net/~lazypower/charms/precise/mediawiki/tests/+merge/202112

(do not edit description out of merge proposal)

[lazypower, 2014-01-20 16:26:10]

Please take a look.

[benjamin.saller, 2014-01-21 17:11:44]

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
File tests/00_setup.sh.moved (right):

https://codereview.appspot.com/54600043/diff/1/tests/00_setup.sh.moved#newcode3
tests/00_setup.sh.moved:3: sudo apt-get install install amulet 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
File tests/100_deploy_cloud.test (right):

https://codereview.appspot.com/54600043/diff/1/tests/100_deploy_cloud.test#ne...
tests/100_deploy_cloud.test:33: search_term =
"\"{}\";".format(mysql_unit.info['public-address'])
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
File tests/10_cache_relation_test.py (right):

https://codereview.appspot.com/54600043/diff/1/tests/10_cache_relation_test.p...
tests/10_cache_relation_test.py:16: # Add mediawiki and mysql db server
This file and the 100_deploy_with_memcache seem redundant. Given that 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
File tests/10_db_relation_test.py (right):

https://codereview.appspot.com/54600043/diff/1/tests/10_db_relation_test.py#n...
tests/10_db_relation_test.py:32: 
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
tests/lib/helper.py:7: 'Accept':
'text/html,application/xhtml+xml,application/xml;q=0.9,*/*',
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
tests/lib/helper.py:15: def standup_unit(deployment, seconds=900):
deploy as a name? This could bring up more than one unit, no?

[lazypower, 2014-01-22 17:24:17]

Please take a look.

[benjamin.saller, 2014-01-22 19:20:03]

LGTM

Thanks for the improvements, this version looked much cleaner than the last.

[Marco Ceppi, 2014-02-04 21:25:14]

See below, LGTM otherwise.

https://codereview.appspot.com/54600043/diff/20001/tests/00_setup.sh
File tests/00_setup.sh (right):

https://codereview.appspot.com/54600043/diff/20001/tests/00_setup.sh#newcode3
tests/00_setup.sh:3: sudo apt-get install install amulet python-requests
This doesn't actually work

[lazypower, 2014-02-14 21:22:07]

Please take a look.

[peter.petrakis, 2014-02-14 22:01:20]

On 2014/02/14 21:22:07, lazypower wrote:
> Please take a look.

New to this service, so old school for now:



=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-06-28 00:01:31 +0000
+++ hooks/config-changed	2014-01-20 05:33:44 +0000
@@ -59,12 +59,16 @@

...
+
+#added to prevent failure when no db relationship exists
+if [ -f /etc/mediawiki/LocalSettings.php ]; then
+    for admin in $admins ; do
+        user=`echo $admin | cut -d: -f1`
+        pass=`echo $admin | cut -d: -f2`


If "admins" is empty this will happily work, the pipe to cut will not error. In
this case I would
exit 1 with meaningful juju-log message.

+        output=`php createAndPromote.php --conf
/etc/mediawiki/LocalSettings.php $user $pass`
+        if [ ! "$output" = "account exists." ] ; then
+            echo $output
+            exit 1
+        fi
+    done
+fi
 


=== added file 'tests/100-deploy'
--- tests/100-deploy	1970-01-01 00:00:00 +0000
+++ tests/100-deploy	2014-02-14 21:21:50 +0000
@@ -0,0 +1,83 @@
+#!/usr/bin/env python3
+import amulet
+from splinter import Browser
+
+seconds = 900
+
+d = amulet.Deployment()
+
+#Setup the charms, and relationships

Inconsistent comment pattern, should have a space between hash and string

+# Perform the setup for the deployment.
+try:
+    d.setup(seconds)
+    #pings every deployed unit
+    d.sentry.wait(seconds)

Same pattern. Also uppercase or lower case for sentence start, either is fine
just be consistent.


=== added file 'tests/100-deploy'
--- tests/100-deploy	1970-01-01 00:00:00 +0000
+++ tests/100-deploy	2014-02-14 21:21:50 +0000
...
+# Validate that the database server was set for the configuration of MediaWiki
+#Set search term for comparison, and cache the flag in the configuration file
+output, code = mw_unit.run("cat /etc/mediawiki/LocalSettings.php \
+        | grep wgDBserver | awk '{printf $3}'")

You can do that entirely in awk, something like awk '/wgDBserver/ && printf $3' 
FILE. If you're so inclined.

...
+memcached_relation = d.sentry.unit['memcached/0'].relation(
+    'cache', 'mediawiki:cache')
+output, code = mw_unit.run("cat /etc/mediawiki/memcached_settings.php \
+    | grep wgMemCachedServers | tr -d \'array\(\)\; | awk '{printf $3}'")

Just call grep or not, arguably a style call, keeping the file to be worked on
as the first argument.

=== removed file 'tests/100_deploy.test'
--- tests/100_deploy.test	2012-01-31 01:27:17 +0000
+++ tests/100_deploy.test	1970-01-01 00:00:00 +0000
@@ -1,62 +0,0 @@
-#!/bin/sh
-
-if [ -z "`which wget`" ] ; then
-    echo SKIP: need wget to run test.
-    exit 100
-fi

I know this was removed but...
backticks are typically used in in bash shell, $() in bash scripts. Tough habit
to break.