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

Proposed by Charles Butler
Status: Merged
Merged at revision: 74
Proposed branch: lp:~lazypower/charms/precise/mediawiki/tests
Merge into: lp:charms/mediawiki
Diff against target: 254 lines (+109/-117)
5 files modified
hooks/config-changed (+13/-9)
tests/00_setup.sh (+13/-0)
tests/100-deploy (+83/-0)
tests/100_deploy.test (+0/-62)
tests/get-unit-info (+0/-46)
To merge this branch: bzr merge lp:~lazypower/charms/precise/mediawiki/tests
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+202112@code.launchpad.net

Description of the change

Testing

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

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

To post a comment you must log in.
Revision history for this message
Charles Butler (lazypower) wrote :

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

Revision history for this message
Benjamin Saller (bcsaller) wrote :

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/

Revision history for this message
Charles Butler (lazypower) wrote :
Revision history for this message
Matt Bruzek (mbruzek) wrote :

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

Revision history for this message
Benjamin Saller (bcsaller) wrote :

LGTM

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

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

Revision history for this message
Marco Ceppi (marcoceppi) wrote :

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/

Revision history for this message
Charles Butler (lazypower) wrote :
Revision history for this message
Peter Petrakis (peter-petrakis) wrote :

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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
59[ ! -f $cfile.old ] || rm $cfile.old59[ ! -f $cfile.old ] || rm $cfile.old
6060
61cd /usr/share/mediawiki/maintenance61cd /usr/share/mediawiki/maintenance
62for admin in $admins ; do62
63 user=`echo $admin | cut -d: -f1`63#added to prevent failure when no db relationship exists
64 pass=`echo $admin | cut -d: -f2`64if [ -f /etc/mediawiki/LocalSettings.php ]; then
65 output=`php createAndPromote.php --conf /etc/mediawiki/LocalSettings.php $user $pass`65 for admin in $admins ; do
66 if [ ! "$output" = "account exists." ] ; then66 user=`echo $admin | cut -d: -f1`
67 echo $output67 pass=`echo $admin | cut -d: -f2`
68 exit 168 output=`php createAndPromote.php --conf /etc/mediawiki/LocalSettings.php $user $pass`
69 fi69 if [ ! "$output" = "account exists." ] ; then
70done70 echo $output
71 exit 1
72 fi
73 done
74fi
7175
=== 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 @@
1#!/bin/bash
2
3#TODO: Some of the python3 packs may not be available in precise.
4
5apt-get install python3-setuptools -y
6
7apt-add-repository ppa:juju/stable -y
8apt-get update
9
10#I have no idea if this is running via python2 or python3, so fetch both
11# copies of the requests lib
12apt-get install juju amulet python3 python3-requests python3-pip phantomjs -y
13pip3 install splinter selenium
0\ No newline at end of file14\ No newline at end of file
115
=== 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 @@
1#!/usr/bin/env python3
2import amulet
3from splinter import Browser
4
5seconds = 900
6
7d = amulet.Deployment()
8
9#Setup the charms, and relationships
10d.add('mysql')
11d.add('memcached')
12d.add('mediawiki')
13d.configure('mediawiki', {'admins': 'tom:swordfish', 'name': 'amulet-wiki'})
14d.relate('mysql:db', 'mediawiki:db')
15d.relate('memcached:cache', 'mediawiki:cache')
16d.expose('mediawiki')
17
18
19# Perform the setup for the deployment.
20try:
21 d.setup(seconds)
22 #pings every deployed unit
23 d.sentry.wait(seconds)
24except amulet.helpers.TimeoutError:
25 message = 'The environment did not setup in %d seconds.' % seconds
26 amulet.raise_status(amulet.SKIP, msg=message)
27except:
28 raise
29
30mw_unit = d.sentry.unit['mediawiki/0']
31mysql_unit = d.sentry.unit['mysql/0']
32
33#############################################################
34# Validate the DB Relationship using Amulet Sentry
35#############################################################
36sql_relation = d.sentry.unit['mysql/0'].relation('db', 'mediawiki:db')
37
38# Validate that the database server was set for the configuration of MediaWiki
39#Set search term for comparison, and cache the flag in the configuration file
40output, code = mw_unit.run("cat /etc/mediawiki/LocalSettings.php \
41 | grep wgDBserver | awk '{printf $3}'")
42
43search_term = "\"{}\";".format(sql_relation['private-address'])
44if search_term != output:
45 message = "Unable to Determine Remote MYSQL configuration, " \
46 "expected: %s, got: %s" % (search_term, output)
47
48 amulet.raise_status(amulet.FAIL, msg=message)
49
50#############################################################
51# Validate the Memcached Relationship using Amulet Sentry
52#############################################################
53memcached_relation = d.sentry.unit['memcached/0'].relation(
54 'cache', 'mediawiki:cache')
55output, code = mw_unit.run("cat /etc/mediawiki/memcached_settings.php \
56 | grep wgMemCachedServers | tr -d \'array\(\)\; | awk '{printf $3}'")
57search_term = "%s:%s" % memcached_relation['private-address'],
58memcached_relation['port']
59
60#############################################################
61# Validate the installation configuration using PhantomJS
62#############################################################
63browser = Browser('phantomjs')
64browser.visit("http://%s/mediawiki/index.php?"
65 "title=Special:UserLogin&returnto=Main+Page" %
66 d.sentry.unit['mediawiki/0'].info['public-address'])
67
68
69title = str(browser.title).find("Log in")
70if title == -1:
71 amulet.raise_status(amulet.FAIL, msg="Failed login test.")
72
73title = str(browser.title).find("amulet-wiki")
74if title == -1:
75 amulet.raise_status(amulet.FAIL, msg="Failed to parse provided title")
76
77browser.fill("wpName", 'tom')
78browser.fill('wpPassword', 'swordfish')
79browser.find_by_id('wpLoginAttempt').first.click()
80
81if not browser.is_text_present('tom'):
82 amulet.raise_status(amulet.FAIL,
83 msg="Failed to login with configured admin")
084
=== 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 @@
1#!/bin/sh
2
3if [ -z "`which wget`" ] ; then
4 echo SKIP: need wget to run test.
5 exit 100
6fi
7
8set -e
9
10teardown() {
11 juju destroy-service memcached
12 juju destroy-service mysql
13 juju destroy-service mediawiki
14 if [ -n "$datadir" ] ; then
15 if [ -f $datadir/passed ]; then
16 rm -r $datadir
17 else
18 echo $datadir preserved
19 fi
20 fi
21}
22trap teardown EXIT
23
24
25juju deploy mediawiki
26juju deploy mysql
27juju deploy memcached
28juju add-relation mediawiki:db mysql:db
29juju add-relation memcached mediawiki
30juju expose mediawiki
31
32for try in `seq 1 600` ; do
33 host=`juju status | tests/get-unit-info mediawiki public-address`
34 if [ -z "$host" ] ; then
35 sleep 1
36 else
37 break
38 fi
39done
40
41if [ -z "$host" ] ; then
42 echo FAIL: status timed out
43 exit 1
44fi
45
46datadir=`mktemp -d ${TMPDIR:-/tmp}/wget.test.XXXXXXX`
47echo INFO: datadir=$datadir
48
49wget --tries=100 --timeout=6 http://$host/ -O - -a $datadir/wget.log | grep -q '<title>' || continue
50
51if [ $try -eq 600 ] ; then
52 echo FAIL: Timed out waiting.
53 exit 1
54fi
55
56touch $datadir/passed
57
58trap - EXIT
59teardown
60
61echo PASS: deployed successfully
62exit 0
630
=== 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 @@
1#!/usr/bin/python
2# machines:
3# 0: {dns-name: ec2-50-17-84-127.compute-1.amazonaws.com, instance-id: i-8c5c3fec}
4# 1: {dns-name: ec2-184-73-102-113.compute-1.amazonaws.com, instance-id: i-14a2c174}
5# 2: {dns-name: ec2-75-101-184-93.compute-1.amazonaws.com, instance-id: i-e0a2c180}
6# services:
7# mysql:
8# charm: local:mysql-11
9# relations: {db: wordpress}
10# units:
11# mysql/0:
12# machine: 2
13# relations:
14# db: {state: up}
15# state: started
16# wordpress:
17# charm: local:wordpress-31
18# exposed: true
19# relations: {db: mysql}
20# units:
21# wordpress/0:
22# machine: 1
23# open-ports: []
24# relations: {}
25# state: null
26
27import yaml
28import sys
29from subprocess import Popen, PIPE
30
31
32def main():
33 d = yaml.safe_load(Popen(['juju','status'],stdout=PIPE).stdout)
34 srv = d.get("services", {}).get(sys.argv[1])
35 if srv is None:
36 return
37 units = srv.get("units", {})
38 if units is None:
39 return
40 item = units.items()[0][1].get(sys.argv[2])
41 if item is None:
42 return
43 print item
44
45if __name__ == "__main__":
46 main()

Subscribers

People subscribed via source and target branches

to all changes: