Merge ~aieri/charm-sudo-pair:functional_fixtures into ~sudo-pair-charmers/charm-sudo-pair:master

[Andrea Ieri]

Rewrite of fixtures and tests

The fixtures in conftest.py are generic enough to be moved to a test template

[Giuseppe Petralia (peppepetra)]

Looks good to me. All tests pass and the code is nice and clean. I agree with Andrea about conftest.py, that can be merged into charm-template into the functional folder.

review: Approve

diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
new file mode 100644
index 0000000..d5de8f4
--- /dev/null
+++ b/tests/functional/conftest.py
@@ -0,0 +1,164 @@
+#!/usr/bin/python3
+
+import pytest
+import json
+import uuid
+import asyncio
+import juju
+from juju.controller import Controller
+from juju.model import Model
+from juju.errors import JujuError
+
+STAT_FILE = "python3 -c \"import json; import os; s=os.stat('%s'); print(json.dumps({'uid': s.st_uid, 'gid': s.st_gid, 'mode': oct(s.st_mode), 'size': s.st_size}))\""  # noqa: E501
+
+
[e-mail]_fixture(scope='module')
+def event_loop(request):
+    '''Override the default pytest event loop to allow for broaded scoped
+    fixtures'''
+    loop = asyncio.get_event_loop_policy().new_event_loop()
+    asyncio.set_event_loop(loop)
+    loop.set_debug(True)
+    yield loop
+    loop.close()
+    asyncio.set_event_loop(None)
+
+
[e-mail](scope='module')
+async def controller():
+    '''Connect to the current controller'''
+    controller = Controller()
+    await controller.connect_current()
+    yield controller
+    await controller.disconnect()
+
+
[e-mail](scope='module')
+async def model(controller):
+    '''This model lives only for the duration of the test'''
+    model_name = str(uuid.uuid4())
+    model = await controller.add_model(model_name)
+    yield model
+    await model.disconnect()
+    await controller.destroy_model(model_name)
+    while model_name in await controller.list_models():
+        await asyncio.sleep(1)
+
+
[e-mail](scope='module')
+async def current_model():
+    '''Returns the current model, does not create or destroy it'''
+    model = Model()
+    await model.connect_current()
+    yield model
+    await model.disconnect()
+
+
[e-mail]
+async def get_app(model):
+    '''Returns the application requested'''
+    async def _get_app(name):
+        try:
+            return model.applications[name]
+        except KeyError:
+            raise JujuError("Cannot find application {}".format(name))
+    return _get_app
+
+
[e-mail]
+async def get_unit(model):
+    '''Returns the requested <app_name>/<unit_number> unit'''
+    async def _get_unit(name):
+        try:
+            (app_name, unit_number) = name.split('/')
+            return model.applications[app_name].units[unit_number]
+        except (KeyError, ValueError):
+            raise JujuError("Cannot find unit {}".format(name))
+    return _get_unit
+
+
[e-mail]
+async def get_entity(model, get_unit, get_app):
+    '''Returns a unit or an application'''
+    async def _get_entity(name):
+        try:
+            return await get_unit(name)
+        except JujuError:
+            try:
+                return await get_app(name)
+            except JujuError as e:
+                raise JujuError("Cannot find entity {}".format(name))
+    return _get_entity
+
+
[e-mail]
+async def run_command(get_unit):
+    '''
+    Runs a command on a unit.
+
+    :param cmd: Command to be run
+    :param target: Unit object or unit name string
+    '''
+    async def _run_command(cmd, target):
+        unit = (
+            target
+            if type(target) is juju.unit.Unit
+            else await get_unit(target)
+        )
+        action = await unit.run(cmd)
+        return action.results
+    return _run_command
+
+
[e-mail]
+async def file_stat(run_command):
+    '''
+    Runs stat on a file
+
+    :param path: File path
+    :param target: Unit object or unit name string
+    '''
+    async def _file_stat(path, target):
+        cmd = STAT_FILE % path
+        results = await run_command(cmd, target)
+        return json.loads(results['Stdout'])
+    return _file_stat
+
+
[e-mail]
+async def file_contents(run_command):
+    '''
+    Returns the contents of a file
+
+    :param path: File path
+    :param target: Unit object or unit name string
+    '''
+    async def _file_contents(path, target):
+        cmd = 'cat {}'.format(path)
+        results = await run_command(cmd, target)
+        return results['Stdout']
+    return _file_contents
+
+
[e-mail]
+async def reconfigure_app(get_app, model):
+    '''Applies a different config to the requested app'''
+    async def _reconfigure_app(cfg, target):
+        application = (
+            target
+            if type(target) is juju.application.Application
+            else await get_app(target)
+        )
+        await application.set_config(cfg)
+        await application.get_config()
+        await model.block_until(lambda: application.status == 'active')
+    return _reconfigure_app
+
+
[e-mail]
+async def create_group(run_command):
+    '''Creates the UNIX group specified'''
+    async def _create_group(group_name, target):
+        cmd = "sudo groupadd %s" % group_name
+        await run_command(cmd, target)
+    return _create_group
diff --git a/tests/functional/test_deploy.py b/tests/functional/test_deploy.py
index 28ac196..6ce4ec3 100644
--- a/tests/functional/test_deploy.py
+++ b/tests/functional/test_deploy.py
@@ -1,38 +1,33 @@
 #!/usr/bin/python3.6
 
-from juju.model import Model
-import asyncio
-import json
 import pytest
 
-
-STAT_FILE = "python3 -c \"import json; import os; s=os.stat('%s'); print(json.dumps({'uid': s.st_uid, 'gid': s.st_gid, 'mode': oct(s.st_mode), 'size': s.st_size}))\""
-
-FILE_CONTENT = "python3 -c \"print(open('%s').read())\""
-
 pytestmark = pytest.mark.asyncio
+SERIES = ['xenial', 'bionic']
 
+############
+# FIXTURES #
+############
 
[e-mail]
-async def model():
-    model = Model()
-    await model.connect_current()
-    yield model
-    await model.disconnect()
 
+# This fixture shouldn't really be in conftest.py since it's specific to this
+# charm
[e-mail](scope='module',
+                params=SERIES)
+async def deploy_app(request, model):
+    '''Deploys the sudo_pair app as a subordinate of ubuntu'''
+    release = request.param
 
[e-mail]
-async def deploy_app(model):
     await model.deploy(
         'ubuntu',
-        application_name='ubuntu',
-        series='bionic',
+        application_name='ubuntu-' + release,
+        series=release,
         channel='stable'
     )
     sudo_pair_app = await model.deploy(
         'local:',
-        application_name='sudo-pair',
-        series='bionic',
+        application_name='sudo-pair-' + release,
+        series=release,
         num_units=0,
         config={
             'bypass_cmds': '/bin/ls',
@@ -41,124 +36,80 @@ async def deploy_app(model):
         }
     )
     await model.add_relation(
-        'ubuntu',
-        'sudo-pair'
+        'ubuntu-' + release,
+        'sudo-pair-' + release
     )
 
     await model.block_until(lambda: sudo_pair_app.status == 'active')
-    return sudo_pair_app
-
-
[e-mail]
-async def get_app(model):
-    for app in model.applications:
-        if app == 'sudo-pair':
-            return model.applications[app]
-
-
[e-mail]
-async def get_unit(model):
-    for app in model.applications:
-        if app == 'ubuntu':
-            return model.applications[app].units[0]
-
-
[e-mail]
-async def run_command(get_unit):
-    async def make_run_command(cmd):
-        action = await get_unit.run(cmd)
-        return action.results
-    return make_run_command
-
-
[e-mail]
-async def file_stat(run_command):
-    async def make_file_stat(path):
-        cmd = STAT_FILE % path
-        results = await run_command(cmd)
-        return json.loads(results['Stdout'])
-    return make_file_stat
-
+    yield sudo_pair_app
+    # no need to cleanup since the model will be be torn down at the end of the
+    # testing
 
[e-mail]
-async def file_contents(run_command):
-    async def make_file_contents(path):
-        cmd = FILE_CONTENT % path
-        results = await run_command(cmd)
-        return results['Stdout']
-    return make_file_contents
 
[e-mail](scope='module')
+async def unit(deploy_app):
+    '''Returns the sudo_pair unit we've deployed'''
+    return deploy_app.units.pop()
 
[e-mail]
-async def reconfigure_app(get_app):
-    async def make_reconfigure_app(cfg):
-        await get_app.set_config(cfg)
-        await get_app.get_config()
-        await asyncio.sleep(10)
-    return make_reconfigure_app
-
-
[e-mail]
-async def create_group(run_command):
-    async def make_create_group(group_name):
-        cmd = "sudo groupadd %s" % group_name
-        await run_command(cmd)
-    return make_create_group
+#########
+# TESTS #
+#########
 
 
 async def test_deploy(deploy_app):
-    status = deploy_app.status
-    assert status == 'active'
-
-
-async def test_sudo_pair_lib(file_stat):
-    sudo_pair_lib_stat = await file_stat("/usr/lib/sudo/sudo_pair.so")
-    assert sudo_pair_lib_stat['size'] > 0
-    assert sudo_pair_lib_stat['gid'] == 0
-    assert sudo_pair_lib_stat['uid'] == 0
-    assert sudo_pair_lib_stat['mode'] == '0o100644'
-
-
-async def test_sudo_approve(file_stat, file_contents):
-    sudo_approve_path = '/usr/bin/sudo_approve'
-    sudo_approve_stat = await file_stat(sudo_approve_path)
-    assert sudo_approve_stat['size'] > 0
-    assert sudo_approve_stat['gid'] == 0
-    assert sudo_approve_stat['uid'] == 0
-    assert sudo_approve_stat['mode'] == '0o100755'
-
-
-async def test_sudo_prompt(file_stat):
-    for prompt_type in ['user', 'pair']:
-        sudo_prompt_stat = await file_stat('/etc/sudo_pair.prompt.' + prompt_type)
-        assert sudo_prompt_stat['size'] > 0
-        assert sudo_prompt_stat['gid'] == 0
-        assert sudo_prompt_stat['uid'] == 0
-        assert sudo_prompt_stat['mode'] == '0o100644'
-
-
-async def test_socket_dir(file_stat):
-    dir_stat = await file_stat('/var/run/sudo_pair')
-    assert dir_stat['gid'] == 0
-    assert dir_stat['uid'] == 0
-    assert dir_stat['mode'] == '0o40644'
-
-
-async def test_sudoers(file_contents):
-    sudoers_content = await file_contents("/etc/sudoers")
+    assert deploy_app.status == 'active'
+
+
[e-mail]("path,expected_stat", [
+    ('/usr/lib/sudo/sudo_pair.so', {
+        'gid': 0,
+        'uid': 0,
+        'mode': '0o100644'}),
+    ('/usr/bin/sudo_approve', {
+        'gid': 0,
+        'uid': 0,
+        'mode': '0o100755'}),
+    ('/etc/sudo_pair.prompt.user', {
+        'gid': 0,
+        'uid': 0,
+        'mode': '0o100644'}),
+    ('/etc/sudo_pair.prompt.pair', {
+        'gid': 0,
+        'uid': 0,
+        'mode': '0o100644'}),
+    ('/var/run/sudo_pair', {
+        'gid': 0,
+        'uid': 0,
+        'mode': '0o40644'})
+    ])
+async def test_stats(path, expected_stat, unit, file_stat):
+    test_stat = await file_stat(path, unit)
+    assert test_stat['size'] > 0
+    assert test_stat['gid'] == expected_stat['gid']
+    assert test_stat['uid'] == expected_stat['uid']
+    assert test_stat['mode'] == expected_stat['mode']
+
+
+async def test_sudoers(file_contents, unit):
+    sudoers_content = await file_contents("/etc/sudoers", unit)
     assert 'Defaults	log_output' in sudoers_content
 
 
-async def test_sudoers_bypass_conf(file_contents):
-    sudoers_bypass_content = await file_contents("/etc/sudoers.d/91-bypass-sudopair-cmds")
+async def test_sudoers_bypass_conf(file_contents, unit):
+    path = "/etc/sudoers.d/91-bypass-sudopair-cmds"
+    sudoers_bypass_content = await file_contents(path=path,
+                                                 target=unit)
     content = '%warthogs ALL = (ALL) NOLOG_OUTPUT: /bin/ls'
     assert content in sudoers_bypass_content
 
 
-async def test_reconfigure(reconfigure_app, file_contents, file_stat):
-    auto_approve = "false"
+async def test_reconfigure(reconfigure_app, file_contents, unit, deploy_app):
+    '''Change a charm config parameter and verify that it has been propagated to
+    the unit'''
     sudo_approve_path = '/usr/bin/sudo_approve'
-    await reconfigure_app({'auto_approve': auto_approve})
-    sudo_approve_content = await file_contents(sudo_approve_path)
+    await reconfigure_app(cfg={'auto_approve': 'false'},
+                          target=deploy_app)
+    sudo_approve_content = await file_contents(path=sudo_approve_path,
+                                               target=unit)
     new_content = 'echo "You can\'t approve your own session."'
     assert new_content in sudo_approve_content