Code review comment for lp:~lazypower/charms/precise/mediawiki/tests

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/