Merge ~peppepetra/charm-sudo-pair:makefile-20.08 into charm-sudo-pair:master

[Giuseppe Petralia]

[Paul Goins (vultaire)]

LGTM. I did notice a couple of things which could get cleaned up and are (maybe, arguably) within scope, but they're not critical.

review: Approve

[Xav Paice (xavpaice)]

LGTM, agreed on the things that could be cleaned up but I don't want to delay this change for those.

review: Approve

diff --git a/.gitignore b/.gitignore
index eda8bea..6f1f367 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,33 +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/
-src/.tox/
-.coverage
+# pycharm
+.idea/
 
 # vi
 .*.swp
 
-# pycharm
-.idea/
-.unit-state.db
-src/.unit-state.db
-
 # version data
 repo-info
+version
 
+# Python builds
+deb_dist/
+dist/
 
-# reports
-report/*
-src/report/*
-
-# virtual env
-venv/*
+# Snaps
+*.snap
 
-# builds
-builds/*
\ No newline at end of file
+# Builds
+.build/
\ No newline at end of file
diff --git a/Makefile b/Makefile
index c7506b2..0a84b5f 100644
--- a/Makefile
+++ b/Makefile
@@ -1,53 +1,80 @@
-PROJECTPATH = $(dir $(realpath $(MAKEFILE_LIST)))
-DIRNAME = $(notdir $(PROJECTPATH:%/=%))
+PYTHON := /usr/bin/python3
 
+PROJECTPATH=$(dir $(realpath $(MAKEFILE_LIST)))
 ifndef CHARM_BUILD_DIR
-    CHARM_BUILD_DIR := /tmp/$(DIRNAME)-builds
-    $(warning Warning CHARM_BUILD_DIR was not set, defaulting to $(CHARM_BUILD_DIR))
+	CHARM_BUILD_DIR=${PROJECTPATH}.build
 endif
+ifndef CHARM_LAYERS_DIR
+	CHARM_LAYERS_DIR=${PROJECTPATH}/layers
+endif
+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 lint - run flake8"
-	@echo " make test - run the functional tests, unittests and lint"
-	@echo " make unittest - 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 submodules - make sure that the submodules are up-to-date"
+	@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 test - run lint, proof, unittests and functional targets"
 	@echo ""
 
-lint:
-	@echo "Running flake8"
-	@tox -e lint
-
-test: lint unittest functional
+clean:
+	@echo "Cleaning files"
+	@git clean -ffXd -e '!.idea'
+	@echo "Cleaning existing build"
+	@rm -rf ${CHARM_BUILD_DIR}/${CHARM_NAME}
 
-functional: build
-	@PYTEST_KEEP_MODEL=$(PYTEST_KEEP_MODEL) \
-	    PYTEST_CLOUD_NAME=$(PYTEST_CLOUD_NAME) \
-	    PYTEST_CLOUD_REGION=$(PYTEST_CLOUD_REGION) \
-	    tox -e functional
+submodules:
+	# @git submodule update --init --recursive
+	@echo "No submodules. Skipping."
 
-unittest:
-	@tox -e unit
+submodules-update:
+	# @git submodule update --init --recursive --remote --merge
+	@echo "No submodules. Skipping."
 
 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 --output-dir $(CHARM_BUILD_DIR) $(PROJECTPATH) --force
+	@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/
 
 release: clean build
-	@echo "Charm is built at $(CHARM_BUILD_DIR)/builds"
+	@echo "Charm is built at ${CHARM_BUILD_DIR}/${CHARM_NAME}"
 
-clean:
-	@echo "Cleaning files"
-	@find $(PROJECTPATH) -iname __pycache__ -exec rm -r {} +
-	@if [ -d $(CHARM_BUILD_DIR)/builds ] ; then rm -r $(CHARM_BUILD_DIR)/builds ; fi
-	@if [ -d $(PROJECTPATH)/.tox ] ; then rm -r $(PROJECTPATH)/.tox ; fi
-	@if [ -d $(PROJECTPATH)/.pytest_cache ] ; then rm -r $(PROJECTPATH)/.pytest_cache ; fi
+lint:
+	@echo "Running lint checks"
+	@cd src && tox -e lint
+
+black:
+	@echo "Reformat files with black"
+	@cd src && tox -e black
+
+proof: build
+	@echo "Running charm proof"
+	@charm proof ${CHARM_BUILD_DIR}/${CHARM_NAME}
+
+unittests:
+	@echo "Running unit tests"
+	@cd src && tox -e unit
+
+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
+.PHONY: help submodules submodules-update clean build release lint black proof unittests functional test
diff --git a/actions/remove-sudopair b/actions/remove-sudopair
deleted file mode 120000
index ff9536b..0000000
--- a/actions/remove-sudopair
+++ /dev/null
@@ -1 +0,0 @@
-./actions.py
\ No newline at end of file
diff --git a/interfaces/.empty b/interfaces/.empty
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/interfaces/.empty
diff --git a/layers/.empty b/layers/.empty
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/layers/.empty
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/actions.py b/src/actions/actions.py
similarity index 100%
rename from actions/actions.py
rename to src/actions/actions.py
diff --git a/src/actions/remove-sudopair b/src/actions/remove-sudopair
new file mode 120000
index 0000000..ff9536b
--- /dev/null
+++ b/src/actions/remove-sudopair
@@ -0,0 +1 @@
+./actions.py
\ No newline at end of file
diff --git a/config.yaml b/src/config.yaml
similarity index 100%
rename from config.yaml
rename to src/config.yaml
diff --git a/files/sudo.prompt.pair b/src/files/sudo.prompt.pair
similarity index 100%
rename from files/sudo.prompt.pair
rename to src/files/sudo.prompt.pair
diff --git a/files/sudo.prompt.user b/src/files/sudo.prompt.user
similarity index 100%
rename from files/sudo.prompt.user
rename to src/files/sudo.prompt.user
diff --git a/files/sudo_pair.so b/src/files/sudo_pair.so
similarity index 100%
rename from files/sudo_pair.so
rename to src/files/sudo_pair.so
Binary files a/files/sudo_pair.so and b/src/files/sudo_pair.so differ
diff --git a/files/sudoers b/src/files/sudoers
similarity index 100%
rename from files/sudoers
rename to src/files/sudoers
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/libsudopair.py b/src/lib/libsudopair.py
similarity index 100%
rename from lib/libsudopair.py
rename to src/lib/libsudopair.py
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/sudo_pair.py b/src/reactive/sudo_pair.py
similarity index 100%
rename from reactive/sudo_pair.py
rename to src/reactive/sudo_pair.py
diff --git a/templates/91-bypass-sudopair-cmds.tmpl b/src/templates/91-bypass-sudopair-cmds.tmpl
similarity index 100%
rename from templates/91-bypass-sudopair-cmds.tmpl
rename to src/templates/91-bypass-sudopair-cmds.tmpl
diff --git a/templates/sudo.conf.tmpl b/src/templates/sudo.conf.tmpl
similarity index 100%
rename from templates/sudo.conf.tmpl
rename to src/templates/sudo.conf.tmpl
diff --git a/templates/sudo_approve.tmpl b/src/templates/sudo_approve.tmpl
similarity index 100%
rename from templates/sudo_approve.tmpl
rename to src/templates/sudo_approve.tmpl
diff --git a/tests/00-unit b/src/tests/00-unit
similarity index 100%
rename from tests/00-unit
rename to src/tests/00-unit
diff --git a/tests/01-functional b/src/tests/01-functional
similarity index 100%
rename from tests/01-functional
rename to src/tests/01-functional
diff --git a/tests/functional/conftest.py b/src/tests/functional/conftest.py
similarity index 99%
rename from tests/functional/conftest.py
rename to src/tests/functional/conftest.py
index aa42a09..68f3ffe 100644
--- a/tests/functional/conftest.py
+++ b/src/tests/functional/conftest.py
@@ -43,7 +43,7 @@ async def model(controller):
     model = await controller.add_model(model_name)
     yield model
     await model.disconnect()
-    if os.getenv('test_preserve_model'):
+    if os.getenv('PYTEST_KEEP_MODEL'):
         return
     await controller.destroy_model(model_name)
     while model_name in await controller.list_models():
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 98%
rename from tests/functional/test_deploy.py
rename to src/tests/functional/test_deploy.py
index 2f0f272..1d531d1 100644
--- a/tests/functional/test_deploy.py
+++ b/src/tests/functional/test_deploy.py
@@ -6,10 +6,10 @@ import pytest
 
 pytestmark = pytest.mark.asyncio
 
-charm_build_dir = os.getenv("CHARM_BUILD_DIR", "..").rstrip("/")
+charm_build_dir = os.getenv("CHARM_BUILD_DIR").rstrip("/")
 
 
-sources = [("local", "{}/builds/sudo-pair".format(charm_build_dir))]
+sources = [("local", "{}/sudo-pair".format(charm_build_dir))]
 
 series = [
     "xenial",
diff --git a/tests/tests.yaml b/src/tests/tests.yaml
similarity index 100%
rename from tests/tests.yaml
rename to src/tests/tests.yaml
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
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_libsudopair.py b/src/tests/unit/test_libsudopair.py
similarity index 100%
rename from tests/unit/test_libsudopair.py
rename to src/tests/unit/test_libsudopair.py
diff --git a/tox.ini b/src/tox.ini
similarity index 62%
rename from tox.ini
rename to src/tox.ini
index fc364de..d07b692 100644
--- a/tox.ini
+++ b/src/tox.ini
@@ -1,51 +1,71 @@
 [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 \
-	   --cov=lib \
-	   --cov=reactive \
-	   --cov=actions \
-	   --cov-report=term \
-	   --cov-report=annotate:report/annotated \
-	   --cov-report=html:report/html
-deps = -r{toxinidir}/tests/unit/requirements.txt
-
-setenv = PYTHONPATH={toxinidir}/lib
-
-[testenv:functional]
+  PYTHONPATH = {toxinidir}:{toxinidir}/lib/:{toxinidir}/hooks/
 passenv =
   HOME
-  CHARM_BUILD_DIR
   PATH
+  CHARM_BUILD_DIR
   PYTEST_KEEP_MODEL
   PYTEST_CLOUD_NAME
   PYTEST_CLOUD_REGION
-commands = pytest -v --ignore {toxinidir}/tests/unit
-deps = -r{toxinidir}/tests/functional/requirements.txt
-
+  PYTEST_MODEL
+  MODEL_SETTINGS
+  HTTP_PROXY
+  HTTPS_PROXY
+  NO_PROXY
+  SNAP_HTTP_PROXY
+  SNAP_HTTPS_PROXY
 
 [testenv:lint]
-commands = flake8
+commands =
+    flake8
+#TODO    black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod)/" .
 deps =
+    black
     flake8
-    flake8-docstrings
-    flake8-import-order
-    pep8-naming
+#TODO    flake8-docstrings
+#TODO    flake8-import-order
+#TODO    pep8-naming
     flake8-colors
 
 [flake8]
-ignore = D100,D103  # Missing docstring in public module/function
 exclude =
     .git,
     __pycache__,
     .tox,
+    charmhelpers,
+    mod,
+    .build
+
 max-line-length = 120
+#TODO max-line-length = 88
 max-complexity = 10
+
+[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