Merge ~jfguedez/juju-lint:fix-unit-tests into juju-lint:master

[Jose Guedez]

Fix current unit tests and formatting

[Jose Guedez (jfguedez)]

Fixes the current broken unit tests:

collected 4 items

tests/test_cli.py::test_pytest PASSED [ 25%]
tests/test_cli.py::test_cli_fixture PASSED [ 50%]
tests/test_jujulint.py::test_flatten_list PASSED [ 75%]
tests/test_jujulint.py::test_map_charms PASSED [100%]

-------------------------------- generated xml file: /home/jguedez/canonical/repos/bootstack/juju-lint/tests/report/junit.xml --------------------------------
--------------------------- generated html file: file:///home/jguedez/canonical/repos/bootstack/juju-lint/tests/report/index.html ----------------------------

----------- coverage: platform linux, python 3.8.5-final-0 -----------
Name Stmts Miss Cover
-------------------------------------------
jujulint/__init__.py 0 0 100%
jujulint/cli.py 79 54 32%
jujulint/cloud.py 170 150 12%
jujulint/config.py 17 0 100%
jujulint/k8s.py 13 13 0%
jujulint/lint.py 388 331 15%
jujulint/logging.py 49 26 47%
jujulint/openstack.py 14 5 64%
jujulint/util.py 20 3 85%
-------------------------------------------
TOTAL 750 582 22%
Coverage annotated source written to dir tests/report/coverage-annotated
Coverage HTML written to dir tests/report/coverage-html

===================================================================== 4 passed in 0.65s ======================================================================
__________________________________________________________________________ summary ___________________________________________________________________________
  unit: commands succeeded
  congratulations :)

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

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

[Linda Guo (lihuiguo)]

lgtm

review: Approve

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

Change successfully merged at revision 25a0f732a2268d9b950f7702f415f9f7ed3a78e4

diff --git a/Makefile b/Makefile
index 03f0a84..917e96b 100644
--- a/Makefile
+++ b/Makefile
@@ -9,4 +9,4 @@ deb-src:
 	debuild -S -sa -I.git -I.tox
 
 test:
-	tox -e py3-test
+	tox -e unit
diff --git a/jujulint/cli.py b/jujulint/cli.py
index 68e7bb2..8892420 100755
--- a/jujulint/cli.py
+++ b/jujulint/cli.py
@@ -36,7 +36,15 @@ class Cli:
         """Create new CLI and configure runtime environment."""
         self.config = Config()
         self.logger = Logger(self.config["logging"]["loglevel"].get())
-        self.version = pkg_resources.require("jujulint")[0].version
+
+        # get the version of the current package if available
+        # this will fail if not running from an installed package
+        # e.g. during unit tests
+        try:
+            self.version = pkg_resources.require("jujulint")[0].version
+        except pkg_resources.DistributionNotFound:
+            self.version = "unknown"
+
         rules_file = self.config["rules"]["file"].get()
         # handle absolute path provided
         if os.path.isfile(rules_file):
diff --git a/jujulint/config.py b/jujulint/config.py
index fb5497a..05b7883 100644
--- a/jujulint/config.py
+++ b/jujulint/config.py
@@ -67,7 +67,7 @@ class Config(Configuration):
         self.parser.add_argument(
             "manual-file",
             metavar="manual-file",
-            nargs='?',
+            nargs="?",
             type=str,
             default=None,
             help=(
diff --git a/jujulint/lint.py b/jujulint/lint.py
index 70f5ae6..ce49398 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -95,7 +95,9 @@ class Linter:
                         )
                     )
                     self.lint_rules["subordinates"][name] = dict(where=where)
-            self.lint_rules["known charms"] = flatten_list(self.lint_rules["known charms"])
+            self.lint_rules["known charms"] = flatten_list(
+                self.lint_rules["known charms"]
+            )
             self.logger.debug(
                 "[{}] [{}/{}] Lint Rules: {}".format(
                     self.cloud_name,
@@ -236,13 +238,21 @@ class Linter:
         if check_value is False:
             self.logger.debug(
                 "[{}] [{}/{}] (PASS) Application {} is correctly using default config for {}.".format(
-                    self.cloud_name, self.controller_name, self.model_name, name, rule,
+                    self.cloud_name,
+                    self.controller_name,
+                    self.model_name,
+                    name,
+                    rule,
                 )
             )
             return True
         self.logger.error(
             "[{}] [{}/{}] (FAIL) Application {} has no manual config for {}.".format(
-                self.cloud_name, self.controller_name, self.model_name, name, rule,
+                self.cloud_name,
+                self.controller_name,
+                self.model_name,
+                name,
+                rule,
             )
         )
         return False
@@ -440,13 +450,17 @@ class Linter:
                         continue
                     self.logger.debug(
                         "[{}] [{}/{}] ... and we are a host, will fallthrough".format(
-                            self.cloud_name, self.controller_name, self.model_name,
+                            self.cloud_name,
+                            self.controller_name,
+                            self.model_name,
                         )
                     )
                 elif where == "all or nothing" and required_sub not in all_or_nothing:
                     self.logger.debug(
                         "[{}] [{}/{}] requirement is 'all or nothing' and was 'nothing'.".format(
-                            self.cloud_name, self.controller_name, self.model_name,
+                            self.cloud_name,
+                            self.controller_name,
+                            self.model_name,
                         )
                     )
                     continue
@@ -455,7 +469,9 @@ class Linter:
                 elif where == "container aware":
                     self.logger.debug(
                         "[{}] [{}/{}] requirement is 'container aware'.".format(
-                            self.cloud_name, self.controller_name, self.model_name,
+                            self.cloud_name,
+                            self.controller_name,
+                            self.model_name,
                         )
                     )
                     if is_container(machine):
@@ -476,7 +492,9 @@ class Linter:
                     )
                     exceptions = []
                     if "exceptions" in self.lint_rules["subordinates"][required_sub]:
-                        exceptions = self.lint_rules["subordinates"][required_sub]["exceptions"]
+                        exceptions = self.lint_rules["subordinates"][required_sub][
+                            "exceptions"
+                        ]
                         self.logger.debug(
                             "[{}] [{}/{}] -> exceptions == {}".format(
                                 self.cloud_name,
@@ -529,14 +547,18 @@ class Linter:
                     if not found:
                         self.logger.debug(
                             "[{}] [{}/{}] -> NOT FOUND".format(
-                                self.cloud_name, self.controller_name, self.model_name,
+                                self.cloud_name,
+                                self.controller_name,
+                                self.model_name,
                             )
                         )
                         for app in self.model.apps_on_machines[machine]:
                             self.model.missing_subs[required_sub].add(app)
                     self.logger.debug(
                         "[{}] [{}/{}] -> continue-ing back out...".format(
-                            self.cloud_name, self.controller_name, self.model_name,
+                            self.cloud_name,
+                            self.controller_name,
+                            self.model_name,
                         )
                     )
                     continue
@@ -552,7 +574,9 @@ class Linter:
                     )
                 self.logger.debug(
                     "[{}] [{}/{}] requirement is 'all' OR we fell through.".format(
-                        self.cloud_name, self.controller_name, self.model_name,
+                        self.cloud_name,
+                        self.controller_name,
+                        self.model_name,
                     )
                 )
                 if required_sub not in present_subs:
@@ -567,7 +591,9 @@ class Linter:
                             continue
                     self.logger.debug(
                         "[{}] [{}/{}] not found.".format(
-                            self.cloud_name, self.controller_name, self.model_name,
+                            self.cloud_name,
+                            self.controller_name,
+                            self.model_name,
                         )
                     )
                     for app in self.model.apps_on_machines[machine]:
@@ -612,7 +638,10 @@ class Linter:
                 if charm not in self.model.charms:
                     self.logger.error(
                         "[{}] Ops charm '{}' in OpenStack model {} on controller {} not found".format(
-                            self.cloud_name, charm, self.model_name, self.controller_name
+                            self.cloud_name,
+                            charm,
+                            self.model_name,
+                            self.controller_name,
                         )
                     )
         elif self.cloud_type == "kubernetes":
@@ -656,7 +685,9 @@ class Linter:
         if self.model.duelling_subs:
             self.logger.error(
                 "[{}] [{}/{}] following subordinates where found on machines more than once:".format(
-                    self.cloud_name, self.controller_name, self.model_name,
+                    self.cloud_name,
+                    self.controller_name,
+                    self.model_name,
                 )
             )
             for sub in self.model.duelling_subs:
@@ -782,7 +813,10 @@ class Linter:
         else:
             self.logger.warn(
                 "[{}] [{}/{}] Could not determine appropriate status key for {}.".format(
-                    self.cloud_name, self.controller_name, self.model_name, name,
+                    self.cloud_name,
+                    self.controller_name,
+                    self.model_name,
+                    name,
                 )
             )
 
@@ -913,7 +947,9 @@ class Linter:
                         "[{}] [{}/{}] Relations data found; assuming a bundle and "
                         "skipping AZ and status checks."
                     ).format(
-                        self.cloud_name, self.model_name, self.controller_name,
+                        self.cloud_name,
+                        self.model_name,
+                        self.controller_name,
                     )
                 )
 
@@ -921,6 +957,8 @@ class Linter:
         else:
             self.logger.warn(
                 "[{}] [{}/{}] Model contains no applications, skipping.".format(
-                    self.cloud_name, self.controller_name, self.model_name,
+                    self.cloud_name,
+                    self.controller_name,
+                    self.model_name,
                 )
             )
diff --git a/tests/conftest.py b/tests/conftest.py
index c95eb69..ef72390 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -27,10 +27,11 @@ def mocked_pkg_resources(monkeypatch):
 
 
 @pytest.fixture
-def cli():
+def cli(monkeypatch):
     """Provide a test instance of the CLI class."""
-    from jujulint.cli import Cli
+    monkeypatch.setattr(sys, "argv", ["juju-lint", "-c", "contrib/canonical-rules.yaml"])
 
+    from jujulint.cli import Cli
     cli = Cli()
 
     return cli
diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
index ba2dbf7..4c27cc1 100644
--- a/tests/test_jujulint.py
+++ b/tests/test_jujulint.py
@@ -16,7 +16,7 @@ def test_flatten_list(utils):
     assert flattened_list == utils.flatten_list(unflattened_list)
 
 
-def test_map_charms(lint):
+def test_map_charms(lint, utils):
     """Test the charm name validation code."""
     applications = {
         "test-app-1": {"charm": "cs:~USER/SERIES/TEST-CHARM12-123"},
@@ -32,5 +32,5 @@ def test_map_charms(lint):
     applications = {
         "test-app1": {"charm": "cs:invalid-charm$"},
     }
-    with pytest.raises(jujulint.lint.InvalidCharmNameError):
+    with pytest.raises(utils.InvalidCharmNameError):
         lint.map_charms(applications)