Merge ~aluria/charm-etckeeper:makefile-20.08 into charm-etckeeper:master

[Alvaro Uria]

[Celia Wang (ziyiwang)]

LGTM.

review: Approve

[Paul Goins (vultaire)]

LGTM. Withholding approval until "make tests" can be run against this branch.

[Paul Goins (vultaire)]

Approved and merged

review: Approve

diff --git a/.gitignore b/.gitignore
index 32e2995..6f1f367 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,22 +1,41 @@
+# Juju files
+.unit-state.db
+.go-cookies
+
+layers/*
+interfaces/*
+
 # Byte-compiled / optimized / DLL files
 __pycache__/
 *.py[cod]
 *$py.class
 
+# Tests files and dir
+.pytest_cache/
+.coverage
+.tox
+report/
+htmlcov/
+
 # Log files
 *.log
 
-.tox/
-.coverage
+# pycharm
+.idea/
 
 # vi
 .*.swp
 
-# pycharm
-.idea/
-
 # version data
 repo-info
+version
+
+# Python builds
+deb_dist/
+dist/
+
+# Snaps
+*.snap
 
-# reports
-report/*
+# Builds
+.build/
\ No newline at end of file
diff --git a/.unit-state.db b/.unit-state.db
deleted file mode 100644
index 014b82c..0000000
Binary files a/.unit-state.db and /dev/null differ
diff --git a/Makefile b/Makefile
index ec627d8..5416279 100644
--- a/Makefile
+++ b/Makefile
@@ -1,64 +1,80 @@
+PYTHON := /usr/bin/python3
+
+PROJECTPATH=$(dir $(realpath $(MAKEFILE_LIST)))
 ifndef CHARM_BUILD_DIR
-	CHARM_BUILD_DIR=/tmp/charm-builds
+	CHARM_BUILD_DIR=${PROJECTPATH}.build
 endif
-
-BUILD_VARS=PYTEST_MODEL=$(PYTEST_MODEL) \
-	   PYTEST_KEEP_MODEL=$(PYTEST_KEEP_MODEL) \
-	   PYTEST_CLOUD_NAME=$(PYTEST_CLOUD_NAME) \
-	   PYTEST_CLOUD_REGION=$(PYTEST_CLOUD_REGION) \
-	   CHARM_BUILD_DIR=$(CHARM_BUILD_DIR)
-
-ifneq ($(PYTEST_SELECT_TESTS),)
-BUILD_VARS+=PYTEST_SELECT_TESTS="$(PYTEST_SELECT_TESTS)"
+ifndef CHARM_LAYERS_DIR
+	CHARM_LAYERS_DIR=${PROJECTPATH}/layers
 endif
-ifneq ($(PYTEST_SELECT_MARKS),)
-BUILD_VARS+=PYTEST_SELECT_MARKS="$(PYTEST_SELECT_MARKS)"
+ifndef CHARM_INTERFACES_DIR
+	CHARM_INTERFACES_DIR=${PROJECTPATH}/interfaces
 endif
+METADATA_FILE="src/metadata.yaml"
+CHARM_NAME=$(shell cat ${PROJECTPATH}/${METADATA_FILE} | grep -E "^name:" | awk '{print $$2}')
 
 help:
 	@echo "This project supports the following targets"
 	@echo ""
 	@echo " make help - show this text"
+	@echo " make clean - remove unneeded files"
 	@echo " make submodules - make sure that the submodules are up-to-date"
-	@echo " make lint - run flake8"
-	@echo " make test - run the unittests and lint"
-	@echo " make unittest - run the tests defined in the unittest subdirectory"
+	@echo " make submodules-update - update submodules to latest changes on remote branch"
+	@echo " make build - build the charm"
+	@echo " make release - run clean, submodules, and build targets"
+	@echo " make lint - run flake8 and black --check"
+	@echo " make black - run black and reformat files"
+	@echo " make proof - run charm proof"
+	@echo " make unittests - run the tests defined in the unittest subdirectory"
 	@echo " make functional - run the tests defined in the functional subdirectory"
-	@echo " make release - build the charm"
-	@echo " make clean - remove unneeded files"
+	@echo " make test - run lint, proof, unittests and functional targets"
 	@echo ""
 
+clean:
+	@echo "Cleaning files"
+	@git clean -ffXd -e '!.idea'
+	@echo "Cleaning existing build"
+	@rm -rf ${CHARM_BUILD_DIR}/${CHARM_NAME}
+
 submodules:
 	@echo "Cloning submodules"
 	@git submodule update --init --recursive
 
-lint:
-	@echo "Running flake8"
-	@tox -e lint
+submodules-update:
+	@echo "Pulling latest updates for submodules"
+	@git submodule update --init --recursive --remote --merge
+
+build:
+	@echo "Building charm to directory ${CHARM_BUILD_DIR}/${CHARM_NAME}"
+	@-git rev-parse --abbrev-ref HEAD > ./src/repo-info
+	@CHARM_LAYERS_DIR=${CHARM_LAYERS_DIR} CHARM_INTERFACES_DIR=${CHARM_INTERFACES_DIR} \
+		TERM=linux CHARM_BUILD_DIR=${CHARM_BUILD_DIR} charm build src/
 
-test: lint unittest functional
+release: clean build
+	@echo "Charm is built at ${CHARM_BUILD_DIR}/${CHARM_NAME}"
 
-unittest:
-	@tox -e unit
+lint:
+	@echo "Running lint checks"
+	@cd src && tox -e lint
 
-functional: build
-	@echo Executing with: $(BUILD_VARS) tox -e functional
-	@$(BUILD_VARS) tox -e functional
+black:
+	@echo "Reformat files with black"
+	@cd src && tox -e black
 
-build:
-	@echo "Building charm to base directory $(CHARM_BUILD_DIR)"
-	@-git describe --tags > ./repo-info
-	@CHARM_LAYERS_DIR=./layers CHARM_INTERFACES_DIR=./interfaces TERM=linux \
-		CHARM_BUILD_DIR=$(CHARM_BUILD_DIR) charm build . --force
+proof: build
+	@echo "Running charm proof"
+	@charm proof ${CHARM_BUILD_DIR}/${CHARM_NAME}
 
-release: clean build
-	@echo "Charm is built at $(JUJU_REPOSITORY)/builds"
+unittests:
+	@echo "Running unit tests"
+	@cd src && tox -e unit
 
-clean:
-	@echo "Cleaning files"
-	@if [ -d .tox ] ; then rm -r .tox ; fi
-	@if [ -d .pytest_cache ] ; then rm -r .pytest_cache ; fi
-	@find . -iname __pycache__ -exec rm -r {} +
+functional: build
+	@echo "Executing functional tests in ${CHARM_BUILD_DIR}"
+	@cd src && CHARM_BUILD_DIR=${CHARM_BUILD_DIR} tox -e func
+
+test: lint proof unittests functional
+	@echo "Tests completed for charm ${CHARM_NAME}."
 
 # The targets below don't depend on a file
-.PHONY: lint test unittest functional build release clean help submodules
+.PHONY: help submodules submodules-update clean build release lint black proof unittests functional test
diff --git a/requirements.txt b/requirements.txt
deleted file mode 100644
index 8462291..0000000
--- a/requirements.txt
+++ /dev/null
@@ -1 +0,0 @@
-# Include python requirements here
diff --git a/README.md b/src/README.md
similarity index 100%
rename from README.md
rename to src/README.md
diff --git a/actions.yaml b/src/actions.yaml
similarity index 100%
rename from actions.yaml
rename to src/actions.yaml
diff --git a/actions/commit b/src/actions/commit
similarity index 100%
rename from actions/commit
rename to src/actions/commit
diff --git a/config.yaml b/src/config.yaml
similarity index 100%
rename from config.yaml
rename to src/config.yaml
diff --git a/icon.svg b/src/icon.svg
similarity index 100%
rename from icon.svg
rename to src/icon.svg
diff --git a/layer.yaml b/src/layer.yaml
similarity index 100%
rename from layer.yaml
rename to src/layer.yaml
diff --git a/lib/lib_charm_etckeeper.py b/src/lib/lib_charm_etckeeper.py
similarity index 96%
rename from lib/lib_charm_etckeeper.py
rename to src/lib/lib_charm_etckeeper.py
index 594299f..7fd1176 100644
--- a/lib/lib_charm_etckeeper.py
+++ b/src/lib/lib_charm_etckeeper.py
@@ -1,8 +1,10 @@
 """The EtcKeeper helper module."""
-from charmhelpers.core import hookenv, unitdata, host, templating
-from charms.reactive.helpers import any_file_changed
 from subprocess import check_call
 
+from charmhelpers.core import hookenv, host, templating, unitdata
+
+from charms.reactive.helpers import any_file_changed
+
 
 class EtcKeeper:
     """The EtcKeeper helper class."""
@@ -58,7 +60,8 @@ class EtcKeeper:
     def bootstrap_etc(self):
         """Set up repo and update KV with status."""
         if self.is_initialized:
-            hookenv.log("Skipping etckeeper init on already-initialised repo.", hookenv.DEBUG)
+            hookenv.log("Skipping etckeeper init on already-initialised repo.",
+                        hookenv.DEBUG)
             return True
         if self.init_repo():
             return True
diff --git a/metadata.yaml b/src/metadata.yaml
similarity index 100%
rename from metadata.yaml
rename to src/metadata.yaml
diff --git a/reactive/charm_etckeeper.py b/src/reactive/charm_etckeeper.py
similarity index 100%
rename from reactive/charm_etckeeper.py
rename to src/reactive/charm_etckeeper.py
index 153b38a..1728526 100644
--- a/reactive/charm_etckeeper.py
+++ b/src/reactive/charm_etckeeper.py
@@ -1,8 +1,10 @@
 """Main reactive charm layer for etckeeper."""
-from lib_charm_etckeeper import EtcKeeper
 from charmhelpers.core import hookenv
+
 from charms.reactive import is_flag_set, when, when_not
 
+from lib_charm_etckeeper import EtcKeeper
+
 helper = EtcKeeper()
 
 
diff --git a/templates/etckeeper.conf.j2 b/src/templates/etckeeper.conf.j2
similarity index 100%
rename from templates/etckeeper.conf.j2
rename to src/templates/etckeeper.conf.j2
diff --git a/tests/functional/conftest.py b/src/tests/functional/conftest.py
similarity index 91%
rename from tests/functional/conftest.py
rename to src/tests/functional/conftest.py
index 104791c..d4fbac0 100644
--- a/tests/functional/conftest.py
+++ b/src/tests/functional/conftest.py
@@ -5,25 +5,29 @@ Reusable pytest fixtures for functional testing.
 Environment variables
 ---------------------
 
-PYTEST_CLOUD_REGION, PYTEST_CLOUD_NAME: cloud name and region to use for juju model creation
+PYTEST_CLOUD_REGION, PYTEST_CLOUD_NAME: cloud name and region to use for juju model
+creation
 
-PYTEST_KEEP_MODEL: if set, the testing model won't be torn down at the end of the testing session
+PYTEST_KEEP_MODEL: if set, the testing model won't be torn down at the end of the
+testing session
 
 """
 
 import asyncio
 import os
-import uuid
-import pytest
 import subprocess
+import uuid
 
 from juju.controller import Controller
+
 from juju_tools import JujuTools
 
+import pytest
+
 
 @pytest.fixture(scope="module")
 def event_loop():
-    """Override the default pytest event loop to allow for fixtures using a broader scope."""
+    """Override default pytest event loop to allow fixtures using a broader scope."""
     loop = asyncio.get_event_loop_policy().new_event_loop()
     asyncio.set_event_loop(loop)
     loop.set_debug(True)
diff --git a/tests/functional/juju_tools.py b/src/tests/functional/juju_tools.py
similarity index 100%
rename from tests/functional/juju_tools.py
rename to src/tests/functional/juju_tools.py
index 9840c79..f14c5d4 100644
--- a/tests/functional/juju_tools.py
+++ b/src/tests/functional/juju_tools.py
@@ -1,7 +1,8 @@
 """Juju tools fixture library."""
+import base64
 import pickle
+
 import juju
-import base64
 
 # from juju.errors import JujuError
 
diff --git a/tests/functional/requirements.txt b/src/tests/functional/requirements.txt
similarity index 100%
rename from tests/functional/requirements.txt
rename to src/tests/functional/requirements.txt
diff --git a/tests/functional/test_deploy.py b/src/tests/functional/test_deploy.py
similarity index 91%
rename from tests/functional/test_deploy.py
rename to src/tests/functional/test_deploy.py
index 3943b20..9c58d73 100644
--- a/tests/functional/test_deploy.py
+++ b/src/tests/functional/test_deploy.py
@@ -1,8 +1,9 @@
 """Functional tests for etckeeper charm."""
 import os
-import pytest
-import subprocess
 import stat
+import subprocess
+
+import pytest
 
 # Treat all tests as coroutines
 pytestmark = pytest.mark.asyncio
@@ -89,7 +90,8 @@ async def test_etckeeper_relate(model, series, app, request):
     """Test relating etckeeper subordinte to ubuntu."""
     application_name = "ubuntu-{}".format(series)
     ubuntu = model.applications[application_name]
-    await model.block_until(lambda: ubuntu.status == "active" or ubuntu.status == "error")
+    await model.block_until(lambda: ubuntu.status == "active"
+                            or ubuntu.status == "error")
     await model.add_relation(app.name, application_name)
 
 
@@ -97,8 +99,10 @@ async def test_charmetckeeper_status(model, app, series):
     """Test the status of the application in the model."""
     application_name = "ubuntu-{}".format(series)
     ubuntu = model.applications[application_name]
-    await model.block_until(lambda: ubuntu.status == "active" or ubuntu.status == "error")
-    await model.block_until(lambda: app.status == "active" or app.status == "error")
+    await model.block_until(lambda: ubuntu.status == "active"
+                            or ubuntu.status == "error")
+    await model.block_until(lambda: app.status == "active"
+                            or app.status == "error")
     assert ubuntu.status != "error"
     assert app.status != "error"
 
diff --git a/tests/unit/conftest.py b/src/tests/unit/conftest.py
similarity index 100%
rename from tests/unit/conftest.py
rename to src/tests/unit/conftest.py
index 20b62ba..0b9f3cd 100644
--- a/tests/unit/conftest.py
+++ b/src/tests/unit/conftest.py
@@ -1,6 +1,7 @@
 #!/usr/bin/python3
 """Unit tests for the etckeeper charm."""
 import mock
+
 import pytest
 
 
diff --git a/tests/unit/example.cfg b/src/tests/unit/example.cfg
similarity index 100%
rename from tests/unit/example.cfg
rename to src/tests/unit/example.cfg
diff --git a/tests/unit/requirements.txt b/src/tests/unit/requirements.txt
similarity index 100%
rename from tests/unit/requirements.txt
rename to src/tests/unit/requirements.txt
diff --git a/tests/unit/test_actions.py b/src/tests/unit/test_actions.py
similarity index 100%
rename from tests/unit/test_actions.py
rename to src/tests/unit/test_actions.py
diff --git a/tests/unit/test_lib.py b/src/tests/unit/test_lib.py
similarity index 100%
rename from tests/unit/test_lib.py
rename to src/tests/unit/test_lib.py
diff --git a/tox.ini b/src/tox.ini
similarity index 52%
rename from tox.ini
rename to src/tox.ini
index 94527af..b3f3a73 100644
--- a/tox.ini
+++ b/src/tox.ini
@@ -1,81 +1,70 @@
 [tox]
 skipsdist=True
-envlist = unit, functional
 skip_missing_interpreters = True
+envlist = lint, unit, func
 
 [testenv]
 basepython = python3
 setenv =
-  PYTHONPATH = .
-
-[testenv:unit]
-commands = pytest -v \
-	    --ignore {toxinidir}/tests/functional \
-	    --ignore {toxinidir}/interfaces \
-	    --ignore {toxinidir}/layers \
-	    --cov=lib \
-	    --cov=reactive \
-	    --cov=actions \
-	    --cov-report=term \
-	    --cov-report=annotate:report/unit/coverage-annotated \
-	    --cov-report=html:report/unit/coverage-html \
-	    --html=report/unit/tests/index.html \
-	    --junitxml=report/unit/junit.xml
-deps = -r{toxinidir}/tests/unit/requirements.txt
-       -r{toxinidir}/requirements.txt
-setenv = PYTHONPATH={toxinidir}/lib
-
-[testenv:functional]
+  PYTHONPATH = {toxinidir}:{toxinidir}/lib/:{toxinidir}/hooks/
 passenv =
   HOME
-  CHARM_BUILD_DIR
   PATH
-  PYTEST_MODEL
+  CHARM_BUILD_DIR
   PYTEST_KEEP_MODEL
   PYTEST_CLOUD_NAME
   PYTEST_CLOUD_REGION
-commands = pytest -x -v \
-           -k {env:PYTEST_SELECT_TESTS:test} \
-           -m "{env:PYTEST_SELECT_MARKS:not excluded}" \
-	    --ignore {toxinidir}/tests/unit \
-	    --ignore {toxinidir}/interfaces \
-	    --ignore {toxinidir}/layers \
-	    --html=report/functional/index.html \
-	    --junitxml=report/functional/junit.xml
-deps = -r{toxinidir}/tests/functional/requirements.txt
-	   -r{toxinidir}/requirements.txt
+  PYTEST_MODEL
+  MODEL_SETTINGS
+  HTTP_PROXY
+  HTTPS_PROXY
+  NO_PROXY
+  SNAP_HTTP_PROXY
+  SNAP_HTTPS_PROXY
 
 [testenv:lint]
-commands = flake8
-deps =
+commands =
     flake8
-    flake8-colors
-    flake8-docstrings
-    pep8-naming
-
-[testenv:lintjunit]
-commands = flake8 --format junit-xml --output-file=report/lint/junit.xml
+    # black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod)/" .
 deps =
+    black
     flake8
-    flake8-colors
     flake8-docstrings
-    flake8-formatter-junit-xml
     flake8-import-order
     pep8-naming
+    flake8-colors
 
 [flake8]
 exclude =
     .git,
     __pycache__,
     .tox,
-    layers,
-    interfaces,
-max-line-length = 120
+    charmhelpers,
+    mod,
+    .build
+
+max-line-length = 88
 max-complexity = 10
 
-[pytest]
-markers =
-    deploy: mark deployment tests to allow running w/o redeploy
-filterwarnings = 
-    ignore::DeprecationWarning
+[testenv:black]
+commands =
+    black --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod)/" .
+deps =
+    black
 
+[testenv:unit]
+commands =
+  pytest -v --ignore {toxinidir}/tests/functional \
+           --cov=lib \
+           --cov=reactive \
+           --cov=actions \
+           --cov=hooks \
+           --cov=src \
+           --cov-report=term \
+           --cov-report=annotate:report/annotated \
+           --cov-report=html:report/html
+deps = -r{toxinidir}/tests/unit/requirements.txt
+
+[testenv:func]
+commands = pytest -v --ignore {toxinidir}/tests/unit
+deps = -r{toxinidir}/tests/functional/requirements.txt