Merge ~vultaire/charm-logrotated:makefile-20.08 into charm-logrotated:master

[Paul Goins]

Imported standard Makefile and tox.ini and fixed up tests

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

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

[Xav Paice (xavpaice)]

This was failing functional tests for me on the attempt to deploy the model.

There were a couple of issues, here's a diff:

diff --git a/src/tests/functional/test_logrotate.py b/src/tests/functional/test_logrotate.py
index 06517c1..936c577 100644
--- a/src/tests/functional/test_logrotate.py
+++ b/src/tests/functional/test_logrotate.py
@@ -8,7 +8,7 @@ import pytest
 pytestmark = pytest.mark.asyncio
 SERIES = ['xenial',
           'bionic',
- pytest.param('cosmic', marks=pytest.mark.xfail(reason='canary')),
+ 'focal',
           ]

@@ -29,7 +29,7 @@ async def deploy_app(request, model):
         channel='stable'
     )
     logrotate_app = await model.deploy(
- '{}/logrotate'.format(os.getenv('CHARM_BUILD_DIR')),
+ '{}/logrotated'.format(os.getenv('CHARM_BUILD_DIR')),
         application_name='logrotate-' + release,
         series=release,
         num_units=0,

review: Needs Fixing

[Paul Goins (vultaire)]

Changed pushed; running functional tests to verify.

[Paul Goins (vultaire)]

Functional tests passed. Ready for review again.

[Xav Paice (xavpaice)]

LGTM

review: Approve

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

Change successfully merged at revision ec9985e94479b34a6b74d40326a1c9126fb40e3f

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/.pre-commit-config.yaml b/.pre-commit-config.yaml
deleted file mode 100644
index 6189d3a..0000000
--- a/.pre-commit-config.yaml
+++ /dev/null
@@ -1,49 +0,0 @@
-repos:
--   repo: git://github.com/pre-commit/pre-commit-hooks
-    sha: v1.1.1
-    hooks:
-    -   id: trailing-whitespace
-        exclude: cherrypy/test/static/index.html
-    -   id: flake8
-        args:
-        # W503 ignored for https://github.com/PyCQA/pycodestyle/issues/498
-        - --ignore=W503
-        # E126 ignored, as yapf will give better identation
-        - --ignore=E126
-    -   id: check-merge-conflict
-    -   id: double-quote-string-fixer
-    -   id: end-of-file-fixer
-    -   id: name-tests-test
-    -   id: check-ast
-    -   id: check-added-large-files
-    -   id: check-byte-order-marker
-    -   id: check-case-conflict
-    -   id: check-json
-        include: .mention-bot
-    -   id: pretty-format-json
-    -   id: check-symlinks
-    -   id: check-yaml
-    -   id: detect-private-key
-#        exclude: cherrypy/test/test.pem
-    - id: requirements-txt-fixer
-
-
--   repo: git://github.com/Lucas-C/pre-commit-hooks
-    sha: v1.1.1
-    hooks:
-    - id: remove-tabs
-
-- repo: https://github.com/pre-commit/mirrors-yapf
-  sha: 'master'  # Use the sha / tag you want to point at
-  hooks:
-    - id: yapf
-
-- repo: https://github.com/pre-commit/mirrors-isort
-  sha: 'master'  # Use the sha / tag you want to point at
-  hooks:
-    - id: isort
-
-- repo: git://github.com/FalconSocial/pre-commit-mirrors-pep257
-  sha: 'master'
-  hooks:
-    - id: pep257
diff --git a/Makefile b/Makefile
index a8c1fd7..5416279 100644
--- a/Makefile
+++ b/Makefile
@@ -1,49 +1,80 @@
+PYTHON := /usr/bin/python3
+
+PROJECTPATH=$(dir $(realpath $(MAKEFILE_LIST)))
+ifndef 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 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
 
-test: unittest functional lint
+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/
 
-unittest:
-	@tox -e unit
+release: clean build
+	@echo "Charm is built at ${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
+lint:
+	@echo "Running lint checks"
+	@cd src && tox -e lint
 
-build:
-	@echo "Building charm to base directory $(JUJU_REPOSITORY)"
-	@-git describe --tags > ./repo-info
-	@LAYER_PATH=./layers INTERFACE_PATH=./interfaces TERM=linux \
-		JUJU_REPOSITORY=$(JUJU_REPOSITORY) charm build . --force
+black:
+	@echo "Reformat files with black"
+	@cd src && tox -e black
 
-release: clean build
-	@echo "Charm is built at $(JUJU_REPOSITORY)/builds"
+proof: build
+	@echo "Running charm proof"
+	@charm proof ${CHARM_BUILD_DIR}/${CHARM_NAME}
 
-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 {} +
+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 submodules
+.PHONY: help submodules submodules-update clean build release lint black proof unittests functional test
diff --git a/actions/update-cronjob b/actions/update-cronjob
deleted file mode 120000
index 405a394..0000000
--- a/actions/update-cronjob
+++ /dev/null
@@ -1 +0,0 @@
-actions.py
\ No newline at end of file
diff --git a/actions/update-logrotate-files b/actions/update-logrotate-files
deleted file mode 120000
index 405a394..0000000
--- a/actions/update-logrotate-files
+++ /dev/null
@@ -1 +0,0 @@
-actions.py
\ No newline at end of file
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/.empty b/src/actions/.empty
similarity index 100%
rename from actions/.empty
rename to src/actions/.empty
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/update-cronjob b/src/actions/update-cronjob
new file mode 120000
index 0000000..405a394
--- /dev/null
+++ b/src/actions/update-cronjob
@@ -0,0 +1 @@
+actions.py
\ No newline at end of file
diff --git a/src/actions/update-logrotate-files b/src/actions/update-logrotate-files
new file mode 120000
index 0000000..405a394
--- /dev/null
+++ b/src/actions/update-logrotate-files
@@ -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/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_cron.py b/src/lib/lib_cron.py
similarity index 100%
rename from lib/lib_cron.py
rename to src/lib/lib_cron.py
index 1478a24..b7d24c4 100644
--- a/lib/lib_cron.py
+++ b/src/lib/lib_cron.py
@@ -87,6 +87,5 @@ def main():
     hookenv.status_set('active', 'Unit is ready.')
 
 
-
 if __name__ == '__main__':
     main()
diff --git a/lib/lib_logrotate.py b/src/lib/lib_logrotate.py
similarity index 100%
rename from lib/lib_logrotate.py
rename to src/lib/lib_logrotate.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/logrotate.py b/src/reactive/logrotate.py
similarity index 100%
rename from reactive/logrotate.py
rename to src/reactive/logrotate.py
diff --git a/requirements.txt b/src/requirements.txt
similarity index 100%
rename from requirements.txt
rename to src/requirements.txt
diff --git a/revision b/src/revision
similarity index 100%
rename from revision
rename to src/revision
diff --git a/tests/functional/conftest.py b/src/tests/functional/conftest.py
similarity index 100%
rename from tests/functional/conftest.py
rename to src/tests/functional/conftest.py
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
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_logrotate.py b/src/tests/functional/test_logrotate.py
similarity index 95%
rename from tests/functional/test_logrotate.py
rename to src/tests/functional/test_logrotate.py
index 100690f..936c577 100644
--- a/tests/functional/test_logrotate.py
+++ b/src/tests/functional/test_logrotate.py
@@ -8,9 +8,10 @@ import pytest
 pytestmark = pytest.mark.asyncio
 SERIES = ['xenial',
           'bionic',
-          pytest.param('cosmic', marks=pytest.mark.xfail(reason='canary')),
+          'focal',
           ]
 
+
 ############
 # FIXTURES #
 ############
@@ -28,7 +29,7 @@ async def deploy_app(request, model):
         channel='stable'
     )
     logrotate_app = await model.deploy(
-        '{}/builds/logrotate'.format(os.getenv('JUJU_REPOSITORY')),
+        '{}/logrotated'.format(os.getenv('CHARM_BUILD_DIR')),
         application_name='logrotate-' + release,
         series=release,
         num_units=0,
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 8520709..b4c0517 100644
--- a/tests/unit/conftest.py
+++ b/src/tests/unit/conftest.py
@@ -5,6 +5,7 @@ import mock
 
 import pytest
 
+
 # If layer options are used, add this to ${fixture}
 # and import layer in logrotate
 @pytest.fixture
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/logrotate_example b/src/tests/unit/logrotate_example
similarity index 100%
rename from tests/unit/logrotate_example
rename to src/tests/unit/logrotate_example
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_logrotate.py b/src/tests/unit/test_logrotate.py
similarity index 100%
rename from tests/unit/test_logrotate.py
rename to src/tests/unit/test_logrotate.py
diff --git a/tox.ini b/src/tox.ini
similarity index 56%
rename from tox.ini
rename to src/tox.ini
index 6bae0d7..78da1be 100644
--- a/tox.ini
+++ b/src/tox.ini
@@ -1,35 +1,33 @@
 [tox]
 skipsdist=True
-envlist = unit, functional
 skip_missing_interpreters = True
+envlist = lint, unit, func
 
 [testenv]
 basepython = python3
 setenv =
-  PYTHONPATH = .
-
-[testenv:unit]
-commands = pytest -o log_cli=true --full-trace  -v --ignore {toxinidir}/tests/functional
-deps = -r{toxinidir}/tests/unit/requirements.txt
-       -r{toxinidir}/requirements.txt
-setenv = PYTHONPATH={toxinidir}/lib
-whitelist_externals = pytest
-
-[testenv:functional]
+  PYTHONPATH = {toxinidir}:{toxinidir}/lib/:{toxinidir}/hooks/
 passenv =
   HOME
-  JUJU_REPOSITORY
   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
-       -r{toxinidir}/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
@@ -41,5 +39,35 @@ exclude =
     .git,
     __pycache__,
     .tox,
-max-line-length = 120
+    charmhelpers,
+    mod,
+    .build
+
+#max-line-length = 88
 max-complexity = 10
+
+# from old tox.ini
+max-line-length = 120
+
+[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