Merge charm-hw-health:improve-dev-workflow into charm-hw-health:master

[Facundo Ciccioli]

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:6e476272c328c0f0739f4103299546aa54a42cae

No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want jenkins to rebuild you need to trigger it yourself):
https://code.launchpad.net/~llama-charmers/charm-hw-health/+git/charm-hw-health/+merge/399589/+edit-commit-message

https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/2/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/53/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/140/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/2//rebuild

review: Needs Fixing (continuous-integration)

[Paul Goins (vultaire)]

This is an interesting patch. I have some small concerns; please take a look.

review: Needs Fixing

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Tom Haddon (mthaddon)]

Abstaining, just claimed the review to take it off the canonical-is-reviewers dashboard, mergebot has been switched to add bootstack-reviewers for future MPs.

review: Abstain

Unmerged commits

6e47627... by Facundo Ciccioli on 2021-03-12

Some docs and funcionality that came in handy while working on this charm

It's the second time I have to learn these stuff, maybe it's useful for
others here.

diff --git a/src/README.md b/src/README.md
index e367400..8b9d4b2 100644
--- a/src/README.md
+++ b/src/README.md
@@ -117,6 +117,47 @@ file a bug (see link below) and it will be added.
 
 Manufacturer option needs to be left in auto mode.
 
+# Functional tests' tricks
+
+The functional tests obey a couple of environment variables meant to facilitate
+a quicker write-execute-modify cycle.
+
+* `TESTS_PRESERVE_MODEL`: Setting this to whatever non-empty string will
+  prevent the tests' code from destroying the test model after the tests have
+  ran (note that not setting this will cause the model to be destroyed in any
+  case, not just on a successful run).
+* `TESTS_USE_MODEL`: Set it to a pre-existent model so that the tests are run
+  in it, instead of creating a new one.
+* `TESTS_JUJU_SETTLE_TIMEOUT_sec`: Set this to the desired timeout when waiting
+  for the model to settle after having performed some action on it (like adding
+  a relation or deploying a unit). Defaults to 600 (10 minutes).
+
+Tox is configured to let those variables through (anything starting with
+`TESTS_`) to the commands it executes. Hence if you're using the `Makefile` for
+instance, you can do this:
+
+```
+$ make TESTS_JUJU_SETTLE_TIMEOUT_sec=1200 functional
+```
+
+Also, for the functional tests only, any positional arguments passed in to tox
+after a double dash (`--`) will be inserted at the end of the `pytest` command.
+This can be very handy to filter out everything except the tests you're working
+on:
+
+```
+$ make TESTS_PRESERVE_MODEL=1 functional    # This will take a long time
+$ . $(find -path '/func/' -name activate)
+(func)$ cd src
+(func)$ TEST_USE_MODEL=$(juju models | awk '/functest-/ {print $1;}' | tr -d \*) \
+    tox -e func -- -k 'test_my_wip_test and bionic'
+
+```
+
+If you're changing you charm's code and not only your test's, you'll have to
+remember to `make build` and `juju upgrade-charm ...` before re-running your
+test.
+
 # Contact Information
 
 Please contact the Nagios charmers via the "Submit a bug" link.
diff --git a/src/tests/functional/conftest.py b/src/tests/functional/conftest.py
index c7c856c..f898e30 100644
--- a/src/tests/functional/conftest.py
+++ b/src/tests/functional/conftest.py
@@ -60,11 +60,15 @@ async def controller():
 @pytest.fixture(scope="module")
 async def model(controller):  # pylint: disable=redefined-outer-name
     """Create model which lives only for the duration of the test."""
-    model_name = "functest-{}".format(uuid.uuid4())
-    _model = await controller.add_model(model_name)
+    model_name = os.getenv("TESTS_USE_MODEL", "functest-{}".format(uuid.uuid4()))
+    if os.getenv("TESTS_USE_MODEL"):
+        model_f = controller.get_model
+    else:
+        model_f = controller.add_model
+    _model = await model_f(model_name)
     yield _model
     await _model.disconnect()
-    if not os.getenv("test_preserve_model"):
+    if not os.getenv("TESTS_PRESERVE_MODEL"):
         await controller.destroy_model(model_name)
         while model_name in await controller.list_models():
             await asyncio.sleep(1)
diff --git a/src/tests/functional/test_hwhealth.py b/src/tests/functional/test_hwhealth.py
index 634c545..5b84b36 100644
--- a/src/tests/functional/test_hwhealth.py
+++ b/src/tests/functional/test_hwhealth.py
@@ -22,7 +22,7 @@ SERIES = [
 CHARM_DIR = dirname(dirname(dirname(abspath(__file__))))
 CHARM_BUILD_DIR = dirname(CHARM_DIR)
 NRPECFG_DIR = "/etc/nagios/nrpe.d"
-DEF_TIMEOUT = 600
+DEF_TIMEOUT = int(os.getenv('TESTS_JUJU_SETTLE_TIMEOUT_sec', '600'))
 # These go along with the hpe repos for the hp* tools
 
 ################
@@ -75,6 +75,10 @@ async def deploy_app(request, model):
     release = request.param
     channel = "stable"
     hw_health_app_name = "hw-health-{}".format(release)
+    if os.getenv("TESTS_USE_MODEL"):
+        yield model.applications[hw_health_app_name]
+        return
+
     hw_health_checksum_app_name = "hw-health-checksum-{}".format(release)
 
     for principal_app in ["ubuntu", "nagios"]:
diff --git a/src/tox.ini b/src/tox.ini
index 7da331e..1acdf41 100644
--- a/src/tox.ini
+++ b/src/tox.ini
@@ -76,5 +76,8 @@ commands =
 deps = -r{toxinidir}/tests/unit/requirements.txt
 
 [testenv:func]
-commands = pytest -v --ignore {toxinidir}/tests/unit
+passenv =
+    {[testenv]passenv}
+    TESTS_*
+commands = pytest -v --ignore {toxinidir}/tests/unit {posargs}
 deps = -r{toxinidir}/tests/functional/requirements.txt