Juju Charms Collection
postgresql package

Merge lp://staging/~stub/charms/precise/postgresql/tests into lp://staging/charms/postgresql

Precise Pangolin (12.04)
tests
Merge into trunk

Proposed by Stuart Bishop on 2014-01-08

Status:	Merged
Merged at revision:	79
Proposed branch:	lp://staging/~stub/charms/precise/postgresql/tests
Merge into:	lp://staging/charms/postgresql
Prerequisite:	lp://staging/~stub/charms/precise/postgresql/cleanups
Diff against target:	147 lines (+45/-21) 6 files modified Makefile (+7/-3) README.md (+28/-13) hooks/hooks.py (+1/-1) hooks/test_hooks.py (+6/-3) revision (+0/-1) tests/01_unittests.test (+3/-0)
To merge this branch:	bzr merge lp://staging/~stub/charms/precise/postgresql/tests
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Marco Ceppi (community)	2014-01-08	Approve on 2014-01-16
Chad Smith (community)		Approve on 2014-01-08
charmers	2014-01-15	Pending
Review via email: mp+200788@code.staging.launchpad.net

Description of the change

Fix some drift from trunk in the brand new unit tests.

I have also hopefully improved some newly added documentation. I can revert this if people think the prose style is less clear.

I'm explicitly setting os.environ['CHARM_DIR'] in test_hooks.py to ensure it is always set to the correct directory, rather than having it set correctly if the tests are invoked from the Makefile.

Revision history for this message

Chad Smith (chad.smith) wrote on 2014-01-08:

+1 Looks great Stuart thanks for the quick merge and merge conflict resolution.

Only one suggestion is a change for the Makefile lint target:

lint:
@flake8 --exclude hooks/charmhelpers hooks # requires python-flakes8

review: Approve

Revision history for this message

Chad Smith (chad.smith) wrote on 2014-01-08:

minor typos in the README:

[2] and minor typo:
+If there are more than one PostgreSQL unit related, the client charm

.. I think it should be

If there is more than on PostgreSQL unit related

[3]
+The client charm needs to watch for state changes in it's

it's -> its

lp://staging/~stub/charms/precise/postgresql/tests updated on 2014-01-09

109. By Stuart Bishop on 2014-01-09: Updates per review

Revision history for this message

Stuart Bishop (stub) wrote on 2014-01-09:

All the above improvements have been made in my branch. Thanks :)

Revision history for this message

Marco Ceppi (marcoceppi) wrote on 2014-01-10:

Our automated testing infrastructure runs with $CHARM_DIR as the path tests are executed from. As such cd ../hooks doesn't work in 01_unittests.test Otherwise this LGTM

review: Needs Fixing

Revision history for this message

Marco Ceppi (marcoceppi) wrote on 2014-01-10:

Once updated,please re-assign review to "~charmers" to have it placed back in the review queue

lp://staging/~stub/charms/precise/postgresql/tests updated on 2014-01-15

110. By Stuart Bishop on 2014-01-15: Correct invocation of unittests for automatic test runner

Revision history for this message

Stuart Bishop (stub) wrote on 2014-01-15:

Changes made, re-review requested.

lp://staging/~stub/charms/precise/postgresql/tests updated on 2014-01-15

111. By Stuart Bishop on 2014-01-15: resolve conflicts

Revision history for this message

Marco Ceppi (marcoceppi) wrote on 2014-01-16:

Hey Stuart, thanks for taking care of that, LGTM +1

Something for future reference, since these will be run very soon in an automated charm tester, consider creating something like 00_setup.test or 00_deps.test that is a dash script which calls sudo apt-get install for any packages that any of the test files need to run. The automated charm testing environment will run in an isolated container as a user with at least sudo privileges. Including this deps/setup test as the first to run will insure your tests run properly.

review: Approve