Code review comment for lp://staging/~ev/charms/xenial/cassandra/snap

Revision history for this message
Stuart Bishop (stub) wrote :

This is all looking good. A few comments inline, but nothing series except for the encrypt() method failing under trusty.

We will want integration tests, which starts with creating TestSnapDeployment(Test3UnitDeployment) in tests/test_integration.py. The tests and harness on this branch are Juju1 specific (I can sort Juju 2.0 mostly by syncing testing/amuletfixture.py to the PostgreSQL charm's version).

The integration tests at the moment will almost certainly fail for snap, as they 'juju run --unit ... nodetool ...' so we either need the charm to create symlinks in /usr/local/bin so the standard tool names work, or update the callsites in the test suite (just 3 or 4 I see). Ideally the snap would use the standard tool names like 'nodetool' rather than 'cassandra.nodetool', but from what I can tell that would mean maintaining a separate snap for each cli tool?

« Back to merge proposal