Merge lp:~lazypower/charms/precise/mediawiki/tests into lp:charms/mediawiki

[Charles Butler]

Testing

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

https://codereview.appspot.com/54600043/

[Charles Butler (lazypower)]

Reviewers: mp+202112_code.launchpad.net,

Message:
Please take a look.

Description:

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)

Please review this at https://codereview.appspot.com/54600043/

Affected files (+270, -4 lines):
   A [revision details]
   M hooks/config-changed
   A tests/00_setup.sh
   A tests/00_setup.sh.moved
   A tests/01_deploy_test.py
   M tests/100_deploy.test
   A tests/100_deploy_cloud.test
   A tests/100_deploy_with_memcached.test
   A tests/10_cache_relation_test.py
   A tests/10_db_relation_test.py
   A tests/__init__.py
   A tests/__init__.py.moved
   A tests/lib.moved/__init__.py
   A tests/lib.moved/helper.py
   A tests/lib/__init__.py
   A tests/lib/helper.py

[Benjamin Saller (bcsaller)]

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#newcode33
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.py#newcode16
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#newcode32
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?

https://codereview.appspot.com/54600043/

[Charles Butler (lazypower)]

Please take a look.

https://codereview.appspot.com/54600043/

[Matt Bruzek (mbruzek)]

I took a look at these changes and I give it a +1 on my review.

[Benjamin Saller (bcsaller)]

LGTM

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

https://codereview.appspot.com/54600043/

[Marco Ceppi (marcoceppi)]

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

https://codereview.appspot.com/54600043/

[Charles Butler (lazypower)]

Please take a look.

https://codereview.appspot.com/54600043/

[Peter Petrakis (peter-petrakis)]

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.

https://codereview.appspot.com/54600043/

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-06-28 00:01:31 +0000
+++ hooks/config-changed	2014-02-14 21:21:57 +0000
@@ -59,12 +59,16 @@
 [ ! -f $cfile.old ] || rm $cfile.old
 
 cd /usr/share/mediawiki/maintenance
-for admin in $admins ; do
-    user=`echo $admin | cut -d: -f1`
-    pass=`echo $admin | cut -d: -f2`
-    output=`php createAndPromote.php --conf /etc/mediawiki/LocalSettings.php $user $pass`
-    if [ ! "$output" = "account exists." ] ; then
-        echo $output
-        exit 1
-    fi
-done
+
+#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`
+        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/00_setup.sh'
--- tests/00_setup.sh	1970-01-01 00:00:00 +0000
+++ tests/00_setup.sh	2014-02-14 21:21:57 +0000
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+#TODO: Some of the python3 packs may not be available in precise.
+
+apt-get install python3-setuptools -y
+
+apt-add-repository ppa:juju/stable -y
+apt-get update
+
+#I have no idea if this is running via python2 or python3, so fetch both
+# copies of the requests lib
+apt-get install juju amulet python3 python3-requests python3-pip phantomjs -y
+pip3 install splinter selenium
\ No newline at end of file

=== added file 'tests/100-deploy'
--- tests/100-deploy	1970-01-01 00:00:00 +0000
+++ tests/100-deploy	2014-02-14 21:21:57 +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
+d.add('mysql')
+d.add('memcached')
+d.add('mediawiki')
+d.configure('mediawiki', {'admins': 'tom:swordfish', 'name': 'amulet-wiki'})
+d.relate('mysql:db', 'mediawiki:db')
+d.relate('memcached:cache', 'mediawiki:cache')
+d.expose('mediawiki')
+
+
+# Perform the setup for the deployment.
+try:
+    d.setup(seconds)
+    #pings every deployed unit
+    d.sentry.wait(seconds)
+except amulet.helpers.TimeoutError:
+    message = 'The environment did not setup in %d seconds.' % seconds
+    amulet.raise_status(amulet.SKIP, msg=message)
+except:
+    raise
+
+mw_unit = d.sentry.unit['mediawiki/0']
+mysql_unit = d.sentry.unit['mysql/0']
+
+#############################################################
+# Validate the DB Relationship using Amulet Sentry
+#############################################################
+sql_relation = d.sentry.unit['mysql/0'].relation('db', 'mediawiki:db')
+
+# 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}'")
+
+search_term = "\"{}\";".format(sql_relation['private-address'])
+if search_term != output:
+    message = "Unable to Determine Remote MYSQL configuration, " \
+        "expected: %s, got: %s" % (search_term, output)
+
+    amulet.raise_status(amulet.FAIL, msg=message)
+
+#############################################################
+# Validate the Memcached Relationship using Amulet Sentry
+#############################################################
+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}'")
+search_term = "%s:%s" % memcached_relation['private-address'],
+memcached_relation['port']
+
+#############################################################
+# Validate the installation configuration using PhantomJS
+#############################################################
+browser = Browser('phantomjs')
+browser.visit("http://%s/mediawiki/index.php?"
+              "title=Special:UserLogin&returnto=Main+Page" %
+              d.sentry.unit['mediawiki/0'].info['public-address'])
+
+
+title = str(browser.title).find("Log in")
+if title == -1:
+    amulet.raise_status(amulet.FAIL, msg="Failed login test.")
+
+title = str(browser.title).find("amulet-wiki")
+if title == -1:
+    amulet.raise_status(amulet.FAIL, msg="Failed to parse provided title")
+
+browser.fill("wpName", 'tom')
+browser.fill('wpPassword', 'swordfish')
+browser.find_by_id('wpLoginAttempt').first.click()
+
+if not browser.is_text_present('tom'):
+    amulet.raise_status(amulet.FAIL,
+                        msg="Failed to login with configured admin")

=== 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
-
-set -e
-
-teardown() {
-    juju destroy-service memcached
-    juju destroy-service mysql
-    juju destroy-service mediawiki
-    if [ -n "$datadir" ] ; then
-        if [ -f $datadir/passed ]; then
-            rm -r $datadir
-        else
-            echo $datadir preserved
-        fi
-    fi
-}
-trap teardown EXIT
-
-
-juju deploy mediawiki
-juju deploy mysql
-juju deploy memcached
-juju add-relation mediawiki:db mysql:db
-juju add-relation memcached mediawiki
-juju expose mediawiki
-
-for try in `seq 1 600` ; do
-    host=`juju status | tests/get-unit-info mediawiki public-address`
-    if [ -z "$host" ] ; then
-        sleep 1 
-    else
-        break
-    fi
-done
-
-if [ -z "$host" ] ; then
-    echo FAIL: status timed out 
-    exit 1
-fi
-
-datadir=`mktemp -d ${TMPDIR:-/tmp}/wget.test.XXXXXXX`
-echo INFO: datadir=$datadir
-
-wget --tries=100 --timeout=6 http://$host/ -O - -a $datadir/wget.log | grep -q '<title>' || continue
-
-if [ $try -eq 600 ] ; then
-    echo FAIL: Timed out waiting.
-    exit 1
-fi
-
-touch $datadir/passed
-
-trap - EXIT
-teardown
-
-echo PASS: deployed successfully
-exit 0

=== removed file 'tests/get-unit-info'
--- tests/get-unit-info	2012-01-31 01:27:17 +0000
+++ tests/get-unit-info	1970-01-01 00:00:00 +0000
@@ -1,46 +0,0 @@
-#!/usr/bin/python
-# machines:
-#   0: {dns-name: ec2-50-17-84-127.compute-1.amazonaws.com, instance-id: i-8c5c3fec}
-#   1: {dns-name: ec2-184-73-102-113.compute-1.amazonaws.com, instance-id: i-14a2c174}
-#   2: {dns-name: ec2-75-101-184-93.compute-1.amazonaws.com, instance-id: i-e0a2c180}
-# services:
-#   mysql:
-#     charm: local:mysql-11
-#     relations: {db: wordpress}
-#     units:
-#       mysql/0:
-#         machine: 2
-#         relations:
-#           db: {state: up}
-#         state: started
-#   wordpress:
-#     charm: local:wordpress-31
-#     exposed: true
-#     relations: {db: mysql}
-#     units:
-#       wordpress/0:
-#         machine: 1
-#         open-ports: []
-#         relations: {}
-#         state: null
-
-import yaml
-import sys
-from subprocess import Popen, PIPE
-
-
-def main():
-    d = yaml.safe_load(Popen(['juju','status'],stdout=PIPE).stdout)
-    srv = d.get("services", {}).get(sys.argv[1])
-    if srv is None:
-        return
-    units = srv.get("units", {})
-    if units is None:
-        return
-    item = units.items()[0][1].get(sys.argv[2])
-    if item is None:
-        return
-    print item
-
-if __name__ == "__main__":
-    main()