Merge ~martin-kalcok/juju-lint:unit-tests into juju-lint:master

Proposed by Martin Kalcok
Status: Merged
Approved by: Eric Chen
Approved revision: e504493c6d8167689398da8d07e6113daad9529d
Merged at revision: 3d7a202f2f005c8fc5e9df2e926b6b5001347458
Proposed branch: ~martin-kalcok/juju-lint:unit-tests
Merge into: juju-lint:master
Diff against target: 2367 lines (+1793/-195)
17 files modified
jujulint/check_spaces.py (+1/-3)
jujulint/cli.py (+1/-1)
jujulint/cloud.py (+3/-4)
jujulint/k8s.py (+9/-19)
jujulint/lint.py (+31/-18)
jujulint/logging.py (+15/-19)
jujulint/openstack.py (+1/-5)
tests/conftest.py (+12/-2)
tests/requirements.txt (+1/-2)
tests/test_check_spaces.py (+347/-0)
tests/test_cli.py (+377/-3)
tests/test_cloud.py (+495/-112)
tests/test_jujulint.py (+225/-7)
tests/test_k8s.py (+29/-0)
tests/test_logging.py (+216/-0)
tests/test_openstack.py (+29/-0)
tox.ini (+1/-0)
Reviewer Review Type Date Requested Status
Eric Chen Approve
Gabriel Cocenza Approve
BootStack Reviewers Pending
Review via email: mp+427760@code.launchpad.net

Commit message

Expanded Unit Test coverage.

Description of the change

This change is mostly about adding unit tests. I also made few minor changes to the main codebase
 where it seemed necessary and where it did not change the functionality. I'll add inline comments to any changes to the main code, explaining why I think it should be changed.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

Thanks for the effort Martin.

I'll review on parts and those comments are regarding the test_check_spaces module

review: Needs Fixing
Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

These comments refers to all other modules with the exception of the logging.py that I'll review tomorrow.

review: Needs Fixing
Revision history for this message
Martin Kalcok (martin-kalcok) wrote :

Thanks for your effort Gabriel. I forgot to save my inline comments explaining the few changes I made to the main codebase. I'll add them now, sorry.

Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

These are my final comments about the logging.py module.

I also would like to discuss what should we do to force coverage (--cov-fail-under=MIN) on next MRs. IMO, we should consider these two approaches:

1 - Fixing the coverage percentage to what we have right now (95.13) on the tox command for unit tests

2 - add a "# pragma: no cover" on the lines missing unit tests with a message TODO to cover in the future and limit it with 100 percentage.

I slightly prefer the second option.

Revision history for this message
Martin Kalcok (martin-kalcok) :
Revision history for this message
Gabriel Cocenza (gabrielcocenza) :
Revision history for this message
Martin Kalcok (martin-kalcok) :
Revision history for this message
Eric Chen (eric-chen) wrote :

LGTM, really good job

There is only small suggestion, please change it, then we can merge it into juju-lint asap.

review: Needs Fixing
Revision history for this message
Martin Kalcok (martin-kalcok) wrote :

I updated code according to your comments Erik. Thanks for your review.

I also marked all currently remaining untested code with "#pragma: no cover" and set minimum 100% coverage in tox.ini so that we can require full test coverage on any future PRs.

Revision history for this message
Eric Chen (eric-chen) wrote :

A obvious little mistake

review: Needs Fixing
Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

I think this PR needs to fix some merge conflicts. Other than that, LGTM.

review: Needs Fixing
Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

LGTM, but it's not passing lint test. Once you use "make dev-environment" it will install pre-commit and start catching this kind of issue.

Thanks!

review: Approve
Revision history for this message
Eric Chen (eric-chen) :
review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 3d7a202f2f005c8fc5e9df2e926b6b5001347458

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/jujulint/check_spaces.py b/jujulint/check_spaces.py
2index 975661d..0564508 100644
3--- a/jujulint/check_spaces.py
4+++ b/jujulint/check_spaces.py
5@@ -23,9 +23,7 @@ class Relation:
6 While Juju does define separate provider and requirer roles, we'll ignore
7 those here.
8 """
9- return set([self.endpoint1, self.endpoint2]) == set(
10- [other.endpoint1, other.endpoint2]
11- )
12+ return {self.endpoint1, self.endpoint2} == {other.endpoint1, other.endpoint2}
13
14 @property
15 def endpoints(self):
16diff --git a/jujulint/cli.py b/jujulint/cli.py
17index 97e5a34..2ea188a 100755
18--- a/jujulint/cli.py
19+++ b/jujulint/cli.py
20@@ -197,5 +197,5 @@ def main():
21 cli.usage()
22
23
24-if __name__ == "__main__":
25+if __name__ == "__main__": # pragma: no cover
26 main()
27diff --git a/jujulint/cloud.py b/jujulint/cloud.py
28index 6d0ec44..490f4d2 100644
29--- a/jujulint/cloud.py
30+++ b/jujulint/cloud.py
31@@ -61,10 +61,9 @@ class Cloud:
32 # instance variables
33 self.cloud_state = {}
34 self.access_method = "local"
35- self.ssh_host = ""
36 self.sudo_user = ""
37 self.hostname = ""
38- self.name = ""
39+ self.name = name
40 self.fabric_config = {}
41 self.lint_rules = lint_rules
42 self.lint_overrides = lint_overrides
43@@ -86,7 +85,6 @@ class Cloud:
44 self.access_method = "ssh"
45 elif access_method == "local":
46 self.hostname = socket.getfqdn()
47- self.name = name
48
49 def run_command(self, command):
50 """Run a command via fabric on the local or remote host."""
51@@ -126,7 +124,8 @@ class Cloud:
52 def run_unit_command(self, target, command):
53 """Run a command on a Juju unit and return the output."""
54
55- def parse_yaml(self, yaml_string):
56+ @staticmethod
57+ def parse_yaml(yaml_string):
58 """Parse YAML using PyYAML."""
59 data = yaml.safe_load_all(yaml_string)
60 return list(data)
61diff --git a/jujulint/k8s.py b/jujulint/k8s.py
62index 53b8c30..9303fe6 100644
63--- a/jujulint/k8s.py
64+++ b/jujulint/k8s.py
65@@ -38,28 +38,18 @@ Todo:
66 from jujulint.cloud import Cloud
67
68
69-class OpenStack(Cloud):
70- """Helper class for interacting with Nagios via the livestatus socket."""
71+class Kubernetes(Cloud):
72+ """Specialized subclass of Cloud with helpers related to Kubernetes."""
73
74 def __init__(self, *args, **kwargs):
75 """Initialise class-local variables and configuration and pass to super."""
76- super(OpenStack, self).__init__(*args, **kwargs)
77-
78- def get_neutron_ports(self):
79- """Get a list of neutron ports."""
80-
81- def get_neutron_routers(self):
82- """Get a list of neutron routers."""
83-
84- def get_neutron_networks(self):
85- """Get a list of neutron networks."""
86-
87- def refresh(self):
88- """Refresh cloud information."""
89- return super(OpenStack, self).refresh()
90+ super(Kubernetes, self).__init__(*args, **kwargs)
91+ self.cloud_type = "kubernetes"
92
93 def audit(self):
94 """Audit OpenStack cloud and run base Cloud audits."""
95- # add specific OpenStack checks here
96- self.logger.debug("Running OpenStack-specific audit steps.")
97- super(OpenStack, self).audit()
98+ # add specific Kubernetes checks here
99+ self.logger.info(
100+ "[{}] Running Kubernetes-specific audit steps.".format(self.name)
101+ )
102+ super(Kubernetes, self).audit()
103diff --git a/jujulint/lint.py b/jujulint/lint.py
104index 8f25617..a931d55 100755
105--- a/jujulint/lint.py
106+++ b/jujulint/lint.py
107@@ -177,12 +177,19 @@ class Linter:
108
109 return
110
111- def atoi(self, val):
112- """Deal with complex number representations as strings, returning a number."""
113- if type(val) != str:
114- return val
115+ @staticmethod
116+ def atoi(val):
117+ """Deal with complex number representations as strings.
118
119- if type(val[-1]) != str:
120+ This method attempts to convert string containing number and a
121+ supported suffix (k,m,g,K,M,G) into a int with appropriate value.
122+ e.g.: "2k" -> 2000
123+ "2K" -> 2048
124+
125+ If the input value does not match the expected format, it is returned
126+ without the change.
127+ """
128+ if type(val) != str:
129 return val
130
131 try:
132@@ -190,13 +197,17 @@ class Linter:
133 except Exception:
134 return val
135
136+ suffix = val[-1]
137 quotient = 1024
138- if val[-1].lower() == val[-1]:
139+ if suffix.islower():
140 quotient = 1000
141
142 conv = {"g": quotient**3, "m": quotient**2, "k": quotient}
143
144- return _int * conv[val[-1].lower()]
145+ if suffix.lower() not in conv:
146+ return val
147+
148+ return _int * conv[suffix.lower()]
149
150 def isset(self, name, check_value, rule, config):
151 """Check if value is set per rule constraints."""
152@@ -293,8 +304,8 @@ class Linter:
153
154 def search(self, app_name, check_value, config_key, app_config):
155 """Scan through the charm config looking for a match using the regex pattern."""
156- actual_value = app_config.get(config_key)
157- if actual_value:
158+ if config_key in app_config:
159+ actual_value = app_config.get(config_key)
160 if re.search(str(check_value), str(actual_value)):
161 self._log_with_header(
162 "Application {} has a valid config for '{}': regex {} found at {}".format(
163@@ -450,7 +461,7 @@ class Linter:
164
165 if self.cloud_type == "openstack":
166 # process openstack config rules
167- if "openstack config" in self.lint_rules:
168+ if "openstack config" in self.lint_rules: # pragma: no cover
169 if charm_name in self.lint_rules["openstack config"]:
170 lint_rules.extend(
171 self.lint_rules["openstack config"][charm_name].items()
172@@ -464,7 +475,7 @@ class Linter:
173 lint_rules,
174 )
175
176- def check_subs(self, machines_data):
177+ def check_subs(self, machines_data): # pragma: no cover
178 """Check the subordinates in the model."""
179 all_or_nothing = set()
180 for machine in self.model.subs_on_machines:
181@@ -790,7 +801,7 @@ class Linter:
182 )
183 except Exception:
184 # FOR NOW: super quick and dirty
185- print(
186+ self.logger.warn(
187 "Exception caught during space check; please check space by hand. {}".format(
188 traceback.format_exc()
189 )
190@@ -1034,7 +1045,7 @@ class Linter:
191 data_d["juju-status"],
192 expected=juju_expected,
193 )
194- else:
195+ else: # pragma: no cover
196 self._log_with_header(
197 "Could not determine Juju status for {}.".format(name),
198 level=logging.WARN,
199@@ -1055,7 +1066,7 @@ class Linter:
200 )
201 for container_name in juju_status["machines"][machine_name].get(
202 "container", []
203- ):
204+ ): # pragma: no cover
205 self.check_status_pair(
206 container_name,
207 "container",
208@@ -1117,7 +1128,7 @@ class Linter:
209 for unit in applications[app_name]["units"]:
210 machine = applications[app_name]["units"][unit]["machine"]
211 machine = machine.split("/")[0]
212- if machine not in self.model.machines_to_az:
213+ if machine not in self.model.machines_to_az: # pragma: no cover
214 self._log_with_header(
215 "{}: Can't find machine {} in machine to AZ mapping data".format(
216 app_name,
217@@ -1164,9 +1175,11 @@ class Linter:
218 parsed_yaml = self.get_main_bundle_doc(parsed_yaml_docs)
219 if parsed_yaml:
220 return self.do_lint(parsed_yaml)
221- self.logger.fubar("Failed to parse YAML from file {}".format(filename))
222+ self.logger.fubar(
223+ "Failed to parse YAML from file {}".format(filename)
224+ ) # pragma: no cover
225
226- def do_lint(self, parsed_yaml):
227+ def do_lint(self, parsed_yaml): # pragma: no cover
228 """Lint parsed YAML."""
229 # Handle Juju 2 vs Juju 1
230 applications = "applications"
231@@ -1260,7 +1273,7 @@ class Linter:
232 if line.startswith("!include"):
233 try:
234 _, rel_path = line.split()
235- except ValueError:
236+ except ValueError: # pragma: no cover
237 self.logger.warn(
238 "invalid include in rules, ignored: '{}'".format(line)
239 )
240diff --git a/jujulint/logging.py b/jujulint/logging.py
241index 1d1586d..7ee3e56 100644
242--- a/jujulint/logging.py
243+++ b/jujulint/logging.py
244@@ -49,27 +49,23 @@ class Logger:
245 console.setFormatter(colour_formatter)
246 self.logger.addHandler(console)
247 if logfile:
248- try:
249- file_logger = colorlog.getLogger("file")
250- plain_formatter = logging.Formatter(
251- format_string, datefmt=date_format
252- )
253- # If we send output to the file logger specifically, don't propagate it
254- # to the root logger as well to avoid duplicate output. So if we want
255- # to only send logging output to the file, you would do this:
256- # logging.getLogger('file').info("message for logfile only")
257- # rather than this:
258- # logging.info("message for console and logfile")
259- file_logger.propagate = False
260+ file_logger = colorlog.getLogger("file")
261+ plain_formatter = logging.Formatter(format_string, datefmt=date_format)
262+ # If we send output to the file logger specifically, don't propagate it
263+ # to the root logger as well to avoid duplicate output. So if we want
264+ # to only send logging output to the file, you would do this:
265+ # logging.getLogger('file').info("message for logfile only")
266+ # rather than this:
267+ # logging.info("message for console and logfile")
268+ file_logger.propagate = False
269
270- file_handler = logging.FileHandler(logfile)
271- file_handler.setFormatter(plain_formatter)
272- self.logger.addHandler(file_handler)
273- file_logger.addHandler(file_handler)
274- except IOError:
275- logging.error("Unable to write to logfile: {}".format(logfile))
276+ file_handler = logging.FileHandler(logfile)
277+ file_handler.setFormatter(plain_formatter)
278+ self.logger.addHandler(file_handler)
279+ file_logger.addHandler(file_handler)
280
281- def fubar(self, msg, exit_code=1):
282+ @staticmethod
283+ def fubar(msg, exit_code=1):
284 """Exit and print to stderr because everything is FUBAR."""
285 sys.stderr.write("E: %s\n" % (msg))
286 sys.exit(exit_code)
287diff --git a/jujulint/openstack.py b/jujulint/openstack.py
288index 398721e..5a87252 100644
289--- a/jujulint/openstack.py
290+++ b/jujulint/openstack.py
291@@ -42,7 +42,7 @@ from jujulint.cloud import Cloud
292
293
294 class OpenStack(Cloud):
295- """Helper class for interacting with Nagios via the livestatus socket."""
296+ """Specialized subclass of Cloud with helpers related to OpenStack."""
297
298 def __init__(self, *args, **kwargs):
299 """Initialise class-local variables and configuration and pass to super."""
300@@ -58,10 +58,6 @@ class OpenStack(Cloud):
301 def get_neutron_networks(self):
302 """Get a list of neutron networks."""
303
304- def refresh(self):
305- """Refresh cloud information."""
306- return super(OpenStack, self).refresh()
307-
308 def audit(self):
309 """Audit OpenStack cloud and run base Cloud audits."""
310 # add specific OpenStack checks here
311diff --git a/tests/conftest.py b/tests/conftest.py
312index b43a273..d82193d 100644
313--- a/tests/conftest.py
314+++ b/tests/conftest.py
315@@ -19,6 +19,8 @@ import pytest
316 test_path = os.path.dirname(os.path.abspath(__file__))
317 sys.path.insert(0, test_path + "/../")
318
319+from jujulint import cloud # noqa: E402
320+
321
322 @pytest.fixture
323 def mocked_pkg_resources(monkeypatch):
324@@ -29,7 +31,7 @@ def mocked_pkg_resources(monkeypatch):
325
326
327 @pytest.fixture
328-def cli(monkeypatch):
329+def cli_instance(monkeypatch):
330 """Provide a test instance of the CLI class."""
331 monkeypatch.setattr(
332 sys, "argv", ["juju-lint", "-c", "contrib/canonical-rules.yaml"]
333@@ -77,7 +79,7 @@ def linter(parser):
334
335
336 @pytest.fixture
337-def cloud():
338+def cloud_instance():
339 """Provide a Cloud instance to test."""
340 from jujulint.cloud import Cloud
341
342@@ -210,3 +212,11 @@ def juju_export_bundle():
343 }
344 ],
345 }
346+
347+
348+@pytest.fixture()
349+def patch_cloud_init(mocker):
350+ """Patch objects needed in Cloud.__init__() method."""
351+ mocker.patch.object(cloud, "Logger")
352+ mocker.patch.object(cloud, "Connection")
353+ mocker.patch.object(cloud.socket, "getfqdn", return_value="localhost")
354diff --git a/tests/requirements.txt b/tests/requirements.txt
355index d31d3fb..ce434c0 100644
356--- a/tests/requirements.txt
357+++ b/tests/requirements.txt
358@@ -1,14 +1,13 @@
359 # Module requirements
360 black
361 colorama
362-flake8
363+flake8<5.0 # some flake8 plugins seem to be currently broken with version 5.0+
364 flake8-colors
365 flake8-docstrings
366 flake8-html
367 isort
368 mock
369 pep8-naming
370-pycodestyle
371 pyflakes
372 pytest
373 pytest-cov
374diff --git a/tests/test_check_spaces.py b/tests/test_check_spaces.py
375new file mode 100644
376index 0000000..9b77e66
377--- /dev/null
378+++ b/tests/test_check_spaces.py
379@@ -0,0 +1,347 @@
380+"""Tests for check_spaces.py module."""
381+from unittest.mock import call
382+
383+import pytest
384+
385+from jujulint import check_spaces
386+
387+
388+def test_relation_init():
389+ """Test initiation of Relation instance."""
390+ ep_1 = "Endpoint 1"
391+ ep_2 = "Endpoint 2"
392+
393+ relation = check_spaces.Relation(ep_1, ep_2)
394+
395+ assert relation.endpoint1 == ep_1
396+ assert relation.endpoint2 == ep_2
397+
398+
399+def test_relation_str():
400+ """Test expected string representation of a Relation class."""
401+ ep_1 = "Endpoint 1"
402+ ep_2 = "Endpoint 2"
403+ expected_str = "Relation({} - {})".format(ep_1, ep_2)
404+
405+ relation = check_spaces.Relation(ep_1, ep_2)
406+
407+ assert str(relation) == expected_str
408+
409+
410+@pytest.mark.parametrize(
411+ "rel_1_ep_1, rel_1_ep_2, rel_2_ep_1, rel_2_ep_2, expected_result",
412+ [
413+ (
414+ "same endpoint 1",
415+ "same endpoint 2",
416+ "same endpoint 1",
417+ "same endpoint 2",
418+ True,
419+ ),
420+ (
421+ "same endpoint 1",
422+ "same endpoint 2",
423+ "same endpoint 1",
424+ "different endpoint 2",
425+ False,
426+ ),
427+ (
428+ "same endpoint 1",
429+ "same endpoint 2",
430+ "different endpoint 1",
431+ "different endpoint 2",
432+ False,
433+ ),
434+ (
435+ "same endpoint 1",
436+ "same endpoint 2",
437+ "different endpoint 1",
438+ "same endpoint 2",
439+ False,
440+ ),
441+ ],
442+)
443+def test_relation_eq(rel_1_ep_1, rel_1_ep_2, rel_2_ep_1, rel_2_ep_2, expected_result):
444+ """Test equality operator of Relation class. Only return true if both endpoints match."""
445+ relation_1 = check_spaces.Relation(rel_1_ep_1, rel_1_ep_2)
446+ relation_2 = check_spaces.Relation(rel_2_ep_1, rel_2_ep_2)
447+
448+ assert (relation_1 == relation_2) == expected_result
449+
450+
451+def test_relation_endpoints_prop():
452+ """Test "endpoints" property of a Relation class."""
453+ ep_1 = "Endpoint 1"
454+ ep_2 = "Endpoint 2"
455+
456+ relation = check_spaces.Relation(ep_1, ep_2)
457+
458+ assert relation.endpoints == [ep_1, ep_2]
459+
460+
461+@pytest.mark.parametrize(
462+ "input_order, output_order",
463+ [
464+ # Input endpoints are already in alphabetical order, output unchanged
465+ (
466+ ["A EP", "A Space", "Z EP", "Z Space"],
467+ ["A EP", "A Space", "Z EP", "Z Space"],
468+ ),
469+ # Input endpoints are not in order, output is alphabetically reordered
470+ (
471+ ["Z EP", "Z Space", "A EP", "A Space"],
472+ ["A EP", "A Space", "Z EP", "Z Space"],
473+ ),
474+ # Input endpoints are the same, no reordering occurs on output
475+ (
476+ ["Z EP", "A Space", "Z EP", "Z Space"],
477+ ["Z EP", "A Space", "Z EP", "Z Space"],
478+ ),
479+ ],
480+)
481+def test_space_mismatch_init(input_order, output_order):
482+ """Test initiation of SpaceMismatch class.
483+
484+ This test also verifies that spaces in SpaceMismatch instance are ordered
485+ alphabetically based on the endpoint name.
486+ """
487+ mismatch_instance = check_spaces.SpaceMismatch(*input_order)
488+
489+ # Assert that endpoints are alphabetically reordered
490+ assert mismatch_instance.endpoint1 == output_order[0]
491+ assert mismatch_instance.space1 == output_order[1]
492+ assert mismatch_instance.endpoint2 == output_order[2]
493+ assert mismatch_instance.space2 == output_order[3]
494+
495+
496+def test_space_mismatch_str():
497+ """Test string representation of a SpaceMismatch class."""
498+ ep_1 = "Endpoint 1"
499+ ep_2 = "Endpoint 2"
500+ space_1 = "Space 1"
501+ space_2 = "Space 2"
502+ expected_str = "SpaceMismatch({} (space {}) != {} (space {}))".format(
503+ ep_1, space_1, ep_2, space_2
504+ )
505+
506+ mismatch_instance = check_spaces.SpaceMismatch(ep_1, space_1, ep_2, space_2)
507+
508+ assert str(mismatch_instance) == expected_str
509+
510+
511+def test_space_mismatch_relation_prop():
512+ """Test relation property of a SpaceMismatch class."""
513+ ep_1 = "Endpoint 1"
514+ ep_2 = "Endpoint 2"
515+ space_1 = "Space 1"
516+ space_2 = "Space 2"
517+
518+ expected_relation = check_spaces.Relation(ep_1, ep_2)
519+
520+ mismatch_instance = check_spaces.SpaceMismatch(ep_1, space_1, ep_2, space_2)
521+
522+ assert mismatch_instance.relation == expected_relation
523+
524+
525+def test_space_mismatch_get_charm_relation():
526+ """Test get_charm_relation method of SpaceMismatch."""
527+ app_1 = "ubuntu_server"
528+ charm_1 = "ubuntu"
529+ app_2 = "ubuntu_nrpe"
530+ charm_2 = "nrpe"
531+ ep_1 = app_1 + ":endpoint_1"
532+ ep_2 = app_2 + ":endpoint_2"
533+ space_1 = "Space 1"
534+ space_2 = "Space 2"
535+
536+ app_map = {app_1: charm_1, app_2: charm_2}
537+
538+ expected_relation = check_spaces.Relation("ubuntu:endpoint_1", "nrpe:endpoint_2")
539+
540+ mismatch_instance = check_spaces.SpaceMismatch(ep_1, space_1, ep_2, space_2)
541+
542+ assert mismatch_instance.get_charm_relation(app_map) == expected_relation
543+
544+
545+@pytest.mark.parametrize("use_cmr", [True, False])
546+def test_find_space_mismatches(use_cmr, mocker):
547+ """Test function find_space_mismatches()."""
548+ sample_yaml = "sample yaml"
549+ app_1 = "ubuntu_server"
550+ app_2 = "ubuntu_nrpe"
551+ space_1 = "space 1"
552+ space_2 = "space 2"
553+ app_endpoint_1 = app_1 + ":endpoint"
554+ app_endpoint_2 = app_2 + ":endpoint"
555+ relation = check_spaces.Relation(
556+ app_endpoint_1, "XModel" if use_cmr else app_endpoint_2
557+ )
558+ app_list = [app_1, app_2]
559+ app_spaces = {app_1: {space_1: "foo"}, app_2: {space_2: "bar"}}
560+
561+ app_list_mock = mocker.patch.object(
562+ check_spaces, "get_juju_applications", return_value=app_list
563+ )
564+ app_spaces_mock = mocker.patch.object(
565+ check_spaces, "get_application_spaces", return_value=app_spaces
566+ )
567+ rel_list_mock = mocker.patch.object(
568+ check_spaces, "get_application_relations", return_value=[relation]
569+ )
570+ rel_space_mock = mocker.patch.object(
571+ check_spaces, "get_relation_space", side_effect=[space_1, space_2]
572+ )
573+
574+ expected_mismatch = [
575+ check_spaces.SpaceMismatch(
576+ relation.endpoint1, space_1, relation.endpoint2, space_2
577+ )
578+ ]
579+
580+ mismatch = check_spaces.find_space_mismatches(sample_yaml, True)
581+ result_pairs = zip(expected_mismatch, mismatch)
582+
583+ app_list_mock.assert_called_once_with(sample_yaml)
584+ app_spaces_mock.assert_called_once_with(app_list, sample_yaml)
585+ rel_list_mock.assert_called_once_with(sample_yaml)
586+ rel_space_mock.assert_has_calls(
587+ [
588+ call(relation.endpoint1, app_spaces),
589+ call(relation.endpoint2, app_spaces),
590+ ]
591+ )
592+ for expected_result, actual_result in result_pairs:
593+ assert str(expected_result) == str(actual_result)
594+
595+
596+def test_get_juju_applications():
597+ """Test parsing applications from yaml status."""
598+ app_1 = "ubuntu"
599+ app_2 = "nrpe"
600+ sample_yaml = {
601+ "applications": {app_1: {"charm-url": "ch:foo"}, app_2: {"charm-url": "ch:bar"}}
602+ }
603+
604+ expected_apps = [app_1, app_2]
605+
606+ apps = check_spaces.get_juju_applications(sample_yaml)
607+
608+ assert apps == expected_apps
609+
610+
611+def test_get_application_spaces(mocker):
612+ """Test function that returns map of applications and their bindings.
613+
614+ This test also verifies that default binding to space "alpha" is added to applications
615+ that do not specify any bindings.
616+ """
617+ logger_mock = mocker.patch.object(check_spaces, "LOGGER")
618+ default_binding = ""
619+ default_space = "custom_default_space"
620+ public_binding = "public"
621+ public_space = "public_space"
622+ app_list = ["ubuntu", "nrpe", "mysql"]
623+ sample_yaml = {
624+ "applications": {
625+ # App with proper bindings
626+ app_list[0]: {
627+ "bindings": {
628+ default_binding: default_space,
629+ public_binding: public_space,
630+ }
631+ },
632+ # App with missing default bindings
633+ app_list[1]: {
634+ "bindings": {
635+ public_binding: public_space,
636+ }
637+ },
638+ # App without any bindings defined
639+ app_list[2]: {},
640+ }
641+ }
642+
643+ expected_app_spaces = {
644+ app_list[0]: {default_binding: default_space, public_binding: public_space},
645+ app_list[1]: {
646+ public_binding: public_space,
647+ },
648+ app_list[2]: {default_binding: "alpha"},
649+ }
650+
651+ app_spaces = check_spaces.get_application_spaces(app_list, sample_yaml)
652+
653+ # Verify that all the bindings for properly defined app were returned
654+ # Verify that default binding was added to app that did not have any bindings defined
655+ # Verify that Warning was logged for app without explicit default binding
656+ # Verify that Warnings were logged for app without any bindings
657+
658+ assert app_spaces == expected_app_spaces
659+ logger_mock.warning.assert_has_calls(
660+ [
661+ call(
662+ "Application %s does not define explicit default binding", app_list[1]
663+ ),
664+ call("Application %s is missing explicit bindings", app_list[2]),
665+ call("Setting default binding of '%s' to alpha", app_list[2]),
666+ ]
667+ )
668+
669+
670+def test_get_application_relations():
671+ """Test function that returns list of relations."""
672+ sample_yaml = {
673+ "relations": [
674+ ["ubuntu:juju-info", "nrpe:general-info"],
675+ ["vault:shared-db", "mysql-innodb-cluster:shared-db"],
676+ ]
677+ }
678+
679+ expected_relations = [
680+ check_spaces.Relation("ubuntu:juju-info", "nrpe:general-info"),
681+ check_spaces.Relation("vault:shared-db", "mysql-innodb-cluster:shared-db"),
682+ ]
683+
684+ relations = check_spaces.get_application_relations(sample_yaml)
685+
686+ assert relations == expected_relations
687+
688+
689+@pytest.mark.parametrize("use_explicit_binding", [True, False])
690+def test_get_relation_space(use_explicit_binding):
691+ """Test getting space for a specific binding."""
692+ app_name = "ubuntu"
693+ interface = "juju_info"
694+ default_space = "alpha"
695+ endpoint = app_name + ":" + interface
696+
697+ app_spaces = {"ubuntu": {"": default_space}}
698+
699+ if use_explicit_binding:
700+ expected_space = "custom_space"
701+ app_spaces["ubuntu"][interface] = expected_space
702+ else:
703+ expected_space = default_space
704+
705+ space = check_spaces.get_relation_space(endpoint, app_spaces)
706+
707+ assert space == expected_space
708+
709+
710+def test_get_relation_space_cmr(mocker):
711+ """Test getting space for cross model relation."""
712+ logger_mock = mocker.patch.object(check_spaces, "LOGGER")
713+ app_name = "ubuntu"
714+ interface = "juju_info"
715+ endpoint = app_name + ":" + interface
716+
717+ app_spaces = {}
718+
719+ space = check_spaces.get_relation_space(endpoint, app_spaces)
720+
721+ assert space == "XModel"
722+ logger_mock.warning.assert_called_once_with(
723+ "Multi-model is not supported yet. Please check "
724+ "if '%s' is from another model",
725+ app_name,
726+ )
727diff --git a/tests/test_cli.py b/tests/test_cli.py
728index c5a9e95..ed63f67 100644
729--- a/tests/test_cli.py
730+++ b/tests/test_cli.py
731@@ -1,7 +1,11 @@
732 #!/usr/bin/python3
733 """Test the CLI."""
734+from logging import WARN
735+from unittest.mock import MagicMock, call
736
737-from jujulint.cli import Cli
738+import pytest
739+
740+from jujulint import cli
741
742
743 def test_pytest():
744@@ -9,6 +13,376 @@ def test_pytest():
745 assert True
746
747
748-def test_cli_fixture(cli):
749+def test_cli_fixture(cli_instance):
750 """Test if the CLI fixture works."""
751- assert isinstance(cli, Cli)
752+ assert isinstance(cli_instance, cli.Cli)
753+
754+
755+@pytest.mark.parametrize("output_format_value", ["text", "json"])
756+def test_cli_init(output_format_value, mocker):
757+ """Test initiation of CLI class."""
758+ logging_mock = mocker.patch.object(cli, "logging")
759+
760+ loglevel_value = "warn"
761+ loglevel = MagicMock()
762+ loglevel.get.return_value = loglevel_value
763+
764+ output_format = MagicMock()
765+ output_format.get.return_value = output_format_value
766+
767+ rules_file_value = "/tmp/rules.yaml"
768+ rules_file = MagicMock()
769+ rules_file.get.return_value = rules_file_value
770+
771+ config = {
772+ "logging": {"loglevel": loglevel},
773+ "format": output_format,
774+ "rules": {"file": rules_file},
775+ }
776+
777+ mocker.patch.object(cli, "Config", return_value=config)
778+ mocker.patch.object(cli.os.path, "isfile", return_value=True)
779+
780+ cli_instance = cli.Cli()
781+
782+ assert cli_instance.logger.logger.level == WARN
783+ assert cli_instance.output_format == output_format_value
784+ assert cli_instance.lint_rules == rules_file_value
785+
786+ if output_format_value != "text":
787+ logging_mock.disable.assert_called_once_with(level=logging_mock.CRITICAL)
788+
789+
790+@pytest.mark.parametrize("version", ["1.0", None])
791+def test_cli_ini_version(version, mocker):
792+ """Test detection of juju-lint version on Cli init."""
793+ require_rvalue = MagicMock()
794+ require_rvalue.version = version
795+ mocker.patch.object(cli, "Config")
796+
797+ if version:
798+ require_mock = mocker.patch.object(
799+ cli.pkg_resources, "require", return_value=[require_rvalue]
800+ )
801+ else:
802+ require_mock = mocker.patch.object(
803+ cli.pkg_resources,
804+ "require",
805+ side_effect=cli.pkg_resources.DistributionNotFound,
806+ )
807+
808+ expected_version = version or "unknown"
809+
810+ cli_instance = cli.Cli()
811+
812+ require_mock.assert_called_once_with("jujulint")
813+ assert cli_instance.version == expected_version
814+
815+
816+@pytest.mark.parametrize("rules_path", ["absolute", "relative", None])
817+def test_cli_init_rules_path(rules_path, mocker):
818+ """Test different methods of loading rules file on Cli init.
819+
820+ methods:
821+ * via absolut path
822+ * via path relative to config dir
823+ * rules file could not be found
824+ """
825+ config_dir = "/tmp/foo"
826+ file_path = "rules.yaml"
827+ rule_file = MagicMock()
828+ rule_file.get.return_value = file_path
829+
830+ config_dict = {
831+ "logging": {"loglevel": MagicMock()},
832+ "format": MagicMock(),
833+ "rules": {"file": rule_file},
834+ }
835+ config = MagicMock()
836+ config.__getitem__.side_effect = config_dict.__getitem__
837+ config.config_dir.return_value = config_dir
838+ mocker.patch.object(cli, "Config", return_value=config)
839+ exit_mock = mocker.patch.object(cli.sys, "exit")
840+
841+ if rules_path == "absolute":
842+ mocker.patch.object(cli.os.path, "isfile", return_value=True)
843+ elif rules_path == "relative":
844+ mocker.patch.object(cli.os.path, "isfile", side_effect=[False, True])
845+ else:
846+ mocker.patch.object(cli.os.path, "isfile", return_value=False)
847+
848+ cli_instance = cli.Cli()
849+
850+ if rules_path == "absolute":
851+ assert cli_instance.lint_rules == file_path
852+ exit_mock.assert_not_called()
853+ elif rules_path == "relative":
854+ assert cli_instance.lint_rules == "{}/{}".format(config_dir, file_path)
855+ exit_mock.assert_not_called()
856+ else:
857+ exit_mock.assert_called_once_with(1)
858+
859+
860+@pytest.mark.parametrize("is_cloud_set", [True, False])
861+def test_cli_cloud_type(cli_instance, is_cloud_set):
862+ """Test cloud_type() property of Cli class."""
863+ cloud_type_value = "openstack"
864+ cloud_type = MagicMock()
865+ cloud_type.get.return_value = cloud_type_value
866+
867+ config = {"cloud-type": cloud_type} if is_cloud_set else {}
868+ cli_instance.config = config
869+
870+ if is_cloud_set:
871+ assert cli_instance.cloud_type == cloud_type_value
872+ else:
873+ assert cli_instance.cloud_type is None
874+
875+
876+@pytest.mark.parametrize("is_file_set", [True, False])
877+def test_cli_manual_file(cli_instance, is_file_set):
878+ """Test manual_file() property of Cli class."""
879+ manual_file_value = "./rules.yaml"
880+ manual_file = MagicMock()
881+ manual_file.get.return_value = manual_file_value
882+
883+ config = {"manual-file": manual_file} if is_file_set else {}
884+ cli_instance.config = config
885+
886+ if is_file_set:
887+ assert cli_instance.manual_file == manual_file_value
888+ else:
889+ assert cli_instance.manual_file is None
890+
891+
892+@pytest.mark.parametrize(
893+ "cloud_type_value, manual_file_value",
894+ [
895+ ("openstack", "rules.yaml"),
896+ (None, None),
897+ ],
898+)
899+def test_cli_startup_message(cli_instance, cloud_type_value, manual_file_value, mocker):
900+ """Test output of a startup message."""
901+ version = "1.0"
902+ config_dir = "/tmp/"
903+ lint_rules = "some rules"
904+
905+ log_level_value = "debug"
906+ log_level = MagicMock()
907+ log_level.get.return_value = log_level_value
908+
909+ cloud_type = MagicMock()
910+ cloud_type.get.return_value = cloud_type_value
911+
912+ manual_file = MagicMock()
913+ manual_file.get.return_value = manual_file_value
914+
915+ config_data = {
916+ "logging": {"loglevel": log_level},
917+ "cloud-type": cloud_type,
918+ "manual-file": manual_file,
919+ }
920+
921+ config = MagicMock()
922+ config.config_dir.return_value = config_dir
923+ config.__getitem__.side_effect = config_data.__getitem__
924+ config.__contains__.side_effect = config_data.__contains__
925+
926+ expected_msg = (
927+ "juju-lint version {} starting...\n\t* Config directory: {}\n"
928+ "\t* Cloud type: {}\n\t* Manual file: {}\n\t* Rules file: {}\n"
929+ "\t* Log level: {}\n"
930+ ).format(
931+ version,
932+ config_dir,
933+ cloud_type_value or "Unknown",
934+ manual_file_value or False,
935+ lint_rules,
936+ log_level_value,
937+ )
938+
939+ cli_instance.version = version
940+ cli_instance.config = config
941+ cli_instance.lint_rules = lint_rules
942+ log_mock = mocker.patch.object(cli_instance, "logger")
943+
944+ assert cli_instance.cloud_type == cloud_type_value
945+ cli_instance.startup_message()
946+
947+ log_mock.info.assert_called_once_with(expected_msg)
948+
949+
950+def test_cli_usage(cli_instance):
951+ """Test usage() method of Cli class."""
952+ config_mock = MagicMock()
953+ cli_instance.config = config_mock
954+
955+ cli_instance.usage()
956+
957+ config_mock.parser.print_help.assert_called_once()
958+
959+
960+def test_cli_audit_file(cli_instance, mocker):
961+ """Test method audit_file() from Cli class."""
962+ filename = "/tmp/bundle.yaml"
963+ rules = "/tmp/rules.yaml"
964+ cloud_type = "openstack"
965+ output_format = "text"
966+ linter_object = MagicMock()
967+
968+ mock_linter = mocker.patch.object(cli, "Linter", return_value=linter_object)
969+ cli_instance.lint_rules = rules
970+ cli_instance.output_format = output_format
971+
972+ cli_instance.audit_file(filename, cloud_type)
973+
974+ mock_linter.assert_called_once_with(
975+ filename, rules, cloud_type=cloud_type, output_format=output_format
976+ )
977+ linter_object.read_rules.assert_called_once()
978+ linter_object.lint_yaml_file.assert_called_once_with(filename)
979+
980+
981+def test_cli_audit_all(cli_instance, mocker):
982+ """Test audit_all() method from Cli class."""
983+ audit_mock = mocker.patch.object(cli_instance, "audit")
984+ write_yaml_mock = mocker.patch.object(cli_instance, "write_yaml")
985+
986+ cloud_data = "cloud data"
987+ clouds_value = ["cloud_1", "cloud_2"]
988+ clouds = MagicMock()
989+ clouds.get.return_value = clouds_value
990+
991+ config_data = {"clouds": clouds}
992+ config = MagicMock()
993+ config.__getitem__.side_effect = config_data.__getitem__
994+
995+ cli_instance.clouds = cloud_data
996+ cli_instance.config = config
997+
998+ cli_instance.audit_all()
999+
1000+ audit_mock.assert_has_calls([call(cloud) for cloud in clouds_value])
1001+ write_yaml_mock.assert_called_once_with(cloud_data, "all-data.yaml")
1002+
1003+
1004+@pytest.mark.parametrize("success", [True, False])
1005+def test_cli_audit(cli_instance, success, mocker):
1006+ """Test audit() method from Cli class."""
1007+ cloud_name = "test cloud"
1008+ lint_rules = "rules.yaml"
1009+ cloud_data = {
1010+ "access": "ssh",
1011+ "sudo": "root",
1012+ "host": "juju.host",
1013+ "type": "openstack",
1014+ }
1015+
1016+ cloud = MagicMock()
1017+ cloud.get.return_value = cloud_data
1018+
1019+ config_data = {"clouds": {cloud_name: cloud}}
1020+
1021+ cloud_state = {"key": "value"}
1022+ mock_openstack_instance = MagicMock()
1023+ mock_openstack_instance.refresh.return_value = success
1024+ mock_openstack_instance.cloud_state = cloud_state
1025+ mock_openstack = mocker.patch.object(
1026+ cli, "OpenStack", return_value=mock_openstack_instance
1027+ )
1028+
1029+ mock_yaml = mocker.patch.object(cli_instance, "write_yaml")
1030+
1031+ mock_logger = MagicMock()
1032+ cli_instance.logger = mock_logger
1033+
1034+ cli_instance.config = config_data
1035+ cli_instance.lint_rules = lint_rules
1036+
1037+ # assert cli_instance.config["clouds"]
1038+ cli_instance.audit(cloud_name=cloud_name)
1039+
1040+ mock_openstack.assert_called_once_with(
1041+ cloud_name,
1042+ access_method=cloud_data["access"],
1043+ ssh_host=cloud_data["host"],
1044+ sudo_user=cloud_data["sudo"],
1045+ lint_rules=lint_rules,
1046+ )
1047+
1048+ if success:
1049+ assert cli_instance.clouds[cloud_name] == cloud_state
1050+ mock_yaml.assert_called_once_with(
1051+ cloud_state, "{}-state.yaml".format(cloud_name)
1052+ )
1053+ mock_openstack_instance.audit.assert_called_once()
1054+ else:
1055+ mock_logger.error.assert_called_once_with(
1056+ "[{}] Failed getting cloud state".format(cloud_name)
1057+ )
1058+
1059+
1060+def test_cli_write_yaml(cli_instance, mocker):
1061+ """Test write_yaml() method from Cli class."""
1062+ yaml_mock = mocker.patch.object(cli, "yaml")
1063+ data = "{'yaml': 'data'}"
1064+ file_name = "dump.yaml"
1065+
1066+ output_folder_value = "/tmp"
1067+ output_folder = MagicMock()
1068+ output_folder.get.return_value = output_folder_value
1069+
1070+ opened_file = MagicMock()
1071+ mock_open = mocker.patch("builtins.open", return_value=opened_file)
1072+
1073+ config = {"output": {"dump": True, "folder": output_folder}}
1074+
1075+ cli_instance.config = config
1076+ cli_instance.write_yaml(data, file_name)
1077+
1078+ mock_open.assert_called_once_with(
1079+ "{}/{}".format(output_folder_value, file_name), "w"
1080+ )
1081+ yaml_mock.dump.assert_called_once_with(data, opened_file)
1082+
1083+
1084+@pytest.mark.parametrize("audit_type", ["file", "all", None])
1085+def test_main(cli_instance, audit_type, mocker):
1086+ """Test main entrypoint of jujulint."""
1087+ mocker.patch.object(cli_instance, "startup_message")
1088+ mocker.patch.object(cli_instance, "usage")
1089+ mocker.patch.object(cli_instance, "audit_file")
1090+ mocker.patch.object(cli_instance, "audit_all")
1091+
1092+ manual_file_value = "bundle.yaml"
1093+ manual_file = MagicMock()
1094+ manual_file.get.return_value = manual_file_value
1095+
1096+ cloud_type_value = "openstack"
1097+ cloud_type = MagicMock()
1098+ cloud_type.get.return_value = cloud_type_value
1099+
1100+ cli_instance.config = {"cloud-type": cloud_type}
1101+
1102+ if audit_type == "file":
1103+ cli_instance.config["manual-file"] = manual_file
1104+ elif audit_type == "all":
1105+ cli_instance.config["clouds"] = ["cloud_1", "cloud_2"]
1106+
1107+ mocker.patch.object(cli, "Cli", return_value=cli_instance)
1108+
1109+ cli.main()
1110+
1111+ if audit_type == "file":
1112+ cli_instance.audit_file.assert_called_once_with(
1113+ manual_file_value, cloud_type=cloud_type_value
1114+ )
1115+ cli_instance.audit_all.assert_not_called()
1116+ elif audit_type == "all":
1117+ cli_instance.audit_all.assert_called_once()
1118+ cli_instance.audit_file.assert_not_called()
1119+ else:
1120+ cli_instance.usage.assert_called_once()
1121+ cli_instance.audit_all.assert_not_called()
1122+ cli_instance.audit_file.assert_not_called()
1123diff --git a/tests/test_cloud.py b/tests/test_cloud.py
1124index 885f319..a3ba423 100644
1125--- a/tests/test_cloud.py
1126+++ b/tests/test_cloud.py
1127@@ -1,129 +1,512 @@
1128 #!/usr/bin/python3
1129-"""Tests for Cloud."""
1130+"""Tests for cloud.py module."""
1131 from subprocess import CalledProcessError
1132-from unittest.mock import call, patch
1133+from unittest.mock import MagicMock, call, patch
1134
1135+import pytest
1136
1137-class TestCloud:
1138- """Test the main Cloud class."""
1139+from jujulint import cloud
1140
1141- @patch("jujulint.cloud.check_output")
1142- def test_get_bundle_no_apps(self, mock_check_out, cloud):
1143- """Models with no apps raises CalledProcessError to export bundle."""
1144- cmd = ["juju", "export-bundle", "-m", "my_controller:controller"]
1145- e = CalledProcessError(1, cmd)
1146- mock_check_out.side_effect = e
1147- cloud.get_juju_bundle("my_controller", "controller")
1148- expected_error_msg = call.error(e)
1149
1150- expected_warn_msg = call.warn(
1151- (
1152- "An error happened to get the bundle on my_controller:controller. "
1153- "If the model doesn't have apps, disconsider this message."
1154- )
1155+@patch("jujulint.cloud.check_output")
1156+def test_get_bundle_no_apps(mock_check_out, cloud_instance):
1157+ """Models with no apps raises CalledProcessError to export bundle."""
1158+ cmd = ["juju", "export-bundle", "-m", "my_controller:controller"]
1159+ e = CalledProcessError(1, cmd)
1160+ mock_check_out.side_effect = e
1161+ cloud_instance.get_juju_bundle("my_controller", "controller")
1162+ expected_error_msg = call.error(e)
1163+
1164+ expected_warn_msg = call.warn(
1165+ (
1166+ "An error happened to get the bundle on my_controller:controller. "
1167+ "If the model doesn't have apps, disconsider this message."
1168 )
1169- assert expected_error_msg in cloud.logger.method_calls
1170- assert expected_warn_msg in cloud.logger.method_calls
1171-
1172- @patch("jujulint.cloud.Cloud.parse_yaml")
1173- @patch("jujulint.cloud.Cloud.run_command")
1174- def test_get_bundle_offer_side(
1175- self, mock_run, mock_parse, cloud, juju_export_bundle
1176- ):
1177- """Test the bundle generated in the offer side."""
1178- # simulate cloud_state with info that came from "get_juju_status"
1179- cloud.cloud_state = {
1180- "my_controller": {
1181- "models": {
1182- "my_model_1": {
1183- "applications": {
1184- "nrpe": {
1185- "charm-origin": "charmhub",
1186- "os": "ubuntu",
1187- "endpoint-bindings": {"": "alpha", "monitors": "alpha"},
1188- }
1189+ )
1190+ assert expected_error_msg in cloud_instance.logger.method_calls
1191+ assert expected_warn_msg in cloud_instance.logger.method_calls
1192+
1193+
1194+@patch("jujulint.cloud.Cloud.parse_yaml")
1195+@patch("jujulint.cloud.Cloud.run_command")
1196+def test_get_bundle_offer_side(
1197+ mock_run, mock_parse, cloud_instance, juju_export_bundle
1198+):
1199+ """Test the bundle generated in the offer side."""
1200+ # simulate cloud_state with info that came from "get_juju_status"
1201+ cloud_instance.cloud_state = {
1202+ "my_controller": {
1203+ "models": {
1204+ "my_model_1": {
1205+ "applications": {
1206+ "nrpe": {
1207+ "charm-origin": "charmhub",
1208+ "os": "ubuntu",
1209+ "endpoint-bindings": {"": "alpha", "monitors": "alpha"},
1210 }
1211- },
1212- "my_model_2": {},
1213- }
1214+ }
1215+ },
1216+ "my_model_2": {},
1217 }
1218 }
1219- mock_parse.return_value = juju_export_bundle["my_model_1"]
1220- # "offers" field exists inside nrpe because of the overlay bundle.
1221- # saas field doesn't exist in the offer side because there is no url.
1222- # don't overwrite information that came from "get_juju_status".
1223- expected_cloud_state = {
1224- "my_controller": {
1225- "models": {
1226- "my_model_1": {
1227- "applications": {
1228- "nrpe": {
1229- "charm": "nrpe",
1230- "charm-origin": "charmhub",
1231- "os": "ubuntu",
1232- "endpoint-bindings": {"": "alpha", "monitors": "alpha"},
1233- "channel": "stable",
1234- "revision": 86,
1235- "offers": {
1236- "nrpe": {
1237- "endpoints": ["monitors"],
1238- "acl": {"admin": "admin"},
1239- }
1240- },
1241- },
1242- "ubuntu": {
1243- "charm": "ubuntu",
1244- "channel": "stable",
1245- "revision": 19,
1246- "num_units": 1,
1247- "to": ["0"],
1248- "constraints": "arch=amd64",
1249+ }
1250+ mock_parse.return_value = juju_export_bundle["my_model_1"]
1251+ # "offers" field exists inside nrpe because of the overlay bundle.
1252+ # saas field doesn't exist in the offer side because there is no url.
1253+ # don't overwrite information that came from "get_juju_status".
1254+ expected_cloud_state = {
1255+ "my_controller": {
1256+ "models": {
1257+ "my_model_1": {
1258+ "applications": {
1259+ "nrpe": {
1260+ "charm": "nrpe",
1261+ "charm-origin": "charmhub",
1262+ "os": "ubuntu",
1263+ "endpoint-bindings": {"": "alpha", "monitors": "alpha"},
1264+ "channel": "stable",
1265+ "revision": 86,
1266+ "offers": {
1267+ "nrpe": {
1268+ "endpoints": ["monitors"],
1269+ "acl": {"admin": "admin"},
1270+ }
1271 },
1272- }
1273- },
1274- "my_model_2": {},
1275- }
1276- }
1277- }
1278- cloud.get_juju_bundle("my_controller", "my_model_1")
1279- assert mock_run.called_once_with(
1280- "juju export-bundle -m my_controller:my_model_1"
1281- )
1282- assert cloud.cloud_state == expected_cloud_state
1283-
1284- @patch("jujulint.cloud.Cloud.parse_yaml")
1285- @patch("jujulint.cloud.Cloud.run_command")
1286- def test_get_bundle_consumer_side(
1287- self, mock_run, mock_parse, cloud, juju_export_bundle
1288- ):
1289- """Test the bundle generated in the consumer side."""
1290- mock_parse.return_value = juju_export_bundle["my_model_2"]
1291- # "offers" field won't exist in the consumer side
1292- # saas field exists because the consumer side shows url
1293- expected_cloud_state = {
1294- "my_controller": {
1295- "models": {
1296- "my_model_1": {},
1297- "my_model_2": {
1298- "applications": {
1299- "nagios": {
1300- "charm": "nagios",
1301- "channel": "stable",
1302- "revision": 49,
1303- "num_units": 1,
1304- "to": ["0"],
1305- "constraints": "arch=amd64",
1306- }
1307 },
1308- "saas": {
1309- "nrpe": {"url": "my_controller:admin/my_model_1.nrpe"}
1310+ "ubuntu": {
1311+ "charm": "ubuntu",
1312+ "channel": "stable",
1313+ "revision": 19,
1314+ "num_units": 1,
1315+ "to": ["0"],
1316+ "constraints": "arch=amd64",
1317 },
1318+ }
1319+ },
1320+ "my_model_2": {},
1321+ }
1322+ }
1323+ }
1324+ cloud_instance.get_juju_bundle("my_controller", "my_model_1")
1325+ assert mock_run.called_once_with("juju export-bundle -m my_controller:my_model_1")
1326+ assert cloud_instance.cloud_state == expected_cloud_state
1327+
1328+
1329+@patch("jujulint.cloud.Cloud.parse_yaml")
1330+@patch("jujulint.cloud.Cloud.run_command")
1331+def test_get_bundle_consumer_side(
1332+ mock_run, mock_parse, cloud_instance, juju_export_bundle
1333+):
1334+ """Test the bundle generated in the consumer side."""
1335+ mock_parse.return_value = juju_export_bundle["my_model_2"]
1336+ # "offers" field won't exist in the consumer side
1337+ # saas field exists because the consumer side shows url
1338+ expected_cloud_state = {
1339+ "my_controller": {
1340+ "models": {
1341+ "my_model_1": {},
1342+ "my_model_2": {
1343+ "applications": {
1344+ "nagios": {
1345+ "charm": "nagios",
1346+ "channel": "stable",
1347+ "revision": 49,
1348+ "num_units": 1,
1349+ "to": ["0"],
1350+ "constraints": "arch=amd64",
1351+ }
1352 },
1353- }
1354+ "saas": {"nrpe": {"url": "my_controller:admin/my_model_1.nrpe"}},
1355+ },
1356 }
1357 }
1358- cloud.get_juju_bundle("my_controller", "my_model_2")
1359- assert mock_run.called_once_with(
1360- "juju export-bundle -m my_controller:my_model_2"
1361+ }
1362+ cloud_instance.get_juju_bundle("my_controller", "my_model_2")
1363+ assert mock_run.called_once_with("juju export-bundle -m my_controller:my_model_2")
1364+ assert cloud_instance.cloud_state == expected_cloud_state
1365+
1366+
1367+@pytest.mark.parametrize(
1368+ "sudo_user, access_method, ssh_host",
1369+ [("", "local", ""), ("root", "ssh", "juju.host")],
1370+)
1371+def test_cloud_init(sudo_user, access_method, ssh_host, mocker):
1372+ """Test Cloud class initialization."""
1373+ logger_mock = MagicMock()
1374+ mocker.patch.object(cloud, "Logger", return_value=logger_mock)
1375+ connection_mock = MagicMock()
1376+ mocker.patch.object(cloud, "Connection", return_value=connection_mock)
1377+ local_fqdn = "localhost"
1378+ mocker.patch.object(cloud.socket, "getfqdn", return_value=local_fqdn)
1379+
1380+ name = "Foo Cloud"
1381+ lint_rules = {"Custom": "ruleset"}
1382+ lint_overrides = {"Override": "rule"}
1383+ cloud_type = "test"
1384+
1385+ cloud_instance = cloud.Cloud(
1386+ name=name,
1387+ lint_rules=lint_rules,
1388+ access_method=access_method,
1389+ ssh_host=ssh_host,
1390+ sudo_user=sudo_user,
1391+ lint_overrides=lint_overrides,
1392+ cloud_type=cloud_type,
1393+ )
1394+
1395+ assert cloud_instance.cloud_state == {}
1396+ assert cloud_instance.access_method == access_method
1397+ assert cloud_instance.sudo_user == sudo_user or ""
1398+ assert cloud_instance.lint_rules == lint_rules
1399+ assert cloud_instance.lint_overrides == lint_overrides
1400+
1401+ if sudo_user:
1402+ assert cloud_instance.fabric_config.get("sudo") == {"user": sudo_user}
1403+ else:
1404+ assert cloud_instance.fabric_config == {}
1405+
1406+ if access_method == "local":
1407+ assert cloud_instance.hostname == local_fqdn
1408+ connection_mock.assert_not_called()
1409+ else:
1410+ assert cloud_instance.hostname == ssh_host
1411+ assert cloud_instance.connection == connection_mock
1412+
1413+
1414+def test_run_local_command(patch_cloud_init, mocker):
1415+ """Test running command when the cloud access method is 'local'."""
1416+ expected_result = ".\n.."
1417+ check_output_mock = mocker.patch.object(
1418+ cloud, "check_output", return_value=expected_result
1419+ )
1420+
1421+ command = "ls -la"
1422+ command_split = command.split()
1423+
1424+ cloud_instance = cloud.Cloud(
1425+ name="local test cloud", access_method="local", cloud_type="test"
1426+ )
1427+ result = cloud_instance.run_command(command)
1428+
1429+ # command is executed locally
1430+ check_output_mock.assert_called_once_with(command_split)
1431+ assert result == expected_result
1432+
1433+
1434+@pytest.mark.parametrize("sudo", [True, False])
1435+def test_run_remote_command(patch_cloud_init, sudo, mocker):
1436+ """Test running command when the cloud access method is 'ssh'.
1437+
1438+ This test has two variants. Executing commands as regular user
1439+ and executing them as root.
1440+ """
1441+ expected_result = MagicMock()
1442+ expected_result.stdout = ".\n.."
1443+
1444+ executor_method = "sudo" if sudo else "run"
1445+
1446+ command = "ls -la"
1447+
1448+ cloud_instance = cloud.Cloud(
1449+ name="remote test cloud",
1450+ access_method="ssh",
1451+ ssh_host="juju.host",
1452+ sudo_user="root" if sudo else "",
1453+ cloud_type="test",
1454+ )
1455+
1456+ mocker.patch.object(
1457+ cloud_instance.connection, executor_method, return_value=expected_result
1458+ )
1459+
1460+ result = cloud_instance.run_command(command)
1461+
1462+ # command was executed remotely as root
1463+ if sudo:
1464+ cloud_instance.connection.sudo.assert_called_once_with(
1465+ command, hide=True, warn=True
1466+ )
1467+ else:
1468+ cloud_instance.connection.run.assert_called_once_with(
1469+ command, hide=True, warn=True
1470+ )
1471+ assert result == expected_result.stdout
1472+
1473+
1474+@pytest.mark.parametrize("sudo", [True, False])
1475+def test_run_remote_command_fail(patch_cloud_init, sudo, mocker):
1476+ """Test error logging when remote command fails.
1477+
1478+ This test has two variants. Executing commands as regular user
1479+ and executing them as root.
1480+ """
1481+ command = "ls -la"
1482+ cloud_name = "remote test cloud"
1483+ executor_method = "sudo" if sudo else "run"
1484+ exception = cloud.SSHException()
1485+ expected_message = "[{}] SSH command {} failed: {}".format(
1486+ cloud_name, command, exception
1487+ )
1488+
1489+ cloud_instance = cloud.Cloud(
1490+ name=cloud_name,
1491+ access_method="ssh",
1492+ ssh_host="juju.host",
1493+ sudo_user="root" if sudo else "",
1494+ cloud_type="test",
1495+ )
1496+
1497+ mocker.patch.object(
1498+ cloud_instance.connection, executor_method, side_effect=exception
1499+ )
1500+
1501+ # reset logger mock to wipe any previous calls
1502+ cloud_instance.logger.reset_mock()
1503+
1504+ result = cloud_instance.run_command(command)
1505+
1506+ # remote command failure was logged
1507+ cloud_instance.logger.error.assert_called_once_with(expected_message)
1508+ assert result is None
1509+
1510+
1511+def test_yaml_loading():
1512+ """Test loading yaml documents into list of dictionaries."""
1513+ yaml_string = "controllers:\n" " ctrl1:\n" " current-model: test-model\n"
1514+ expected_output = [{"controllers": {"ctrl1": {"current-model": "test-model"}}}]
1515+
1516+ assert cloud.Cloud.parse_yaml(yaml_string) == expected_output
1517+
1518+
1519+@pytest.mark.parametrize("success", [True, False])
1520+def test_get_juju_controllers(patch_cloud_init, success, mocker):
1521+ """Test method that retrieves a list of juju controllers.
1522+
1523+ This test also verifies behavior in case the command to fetch controllers fails.
1524+ """
1525+ controller_name = "foo"
1526+ controller_config = {
1527+ "current-model": "default",
1528+ "user": "admin",
1529+ "access": "superuser",
1530+ }
1531+ controller_list = [{"controllers": {controller_name: controller_config}}]
1532+
1533+ mocker.patch.object(cloud.Cloud, "run_command", return_value=success)
1534+ mocker.patch.object(cloud.Cloud, "parse_yaml", return_value=controller_list)
1535+
1536+ cloud_instance = cloud.Cloud(name="Test cloud")
1537+
1538+ result = cloud_instance.get_juju_controllers()
1539+
1540+ if success:
1541+ assert result
1542+ assert (
1543+ cloud_instance.cloud_state[controller_name]["config"] == controller_config
1544+ )
1545+ else:
1546+ assert not result
1547+ assert controller_name not in cloud_instance.cloud_state
1548+ cloud_instance.logger.error.assert_called_once_with(
1549+ "[{}] Could not get controller list".format(cloud_instance.name)
1550+ )
1551+
1552+
1553+@pytest.mark.parametrize("success", [True, False])
1554+def test_get_juju_models(patch_cloud_init, success, mocker):
1555+ """Test methods that retrieves a list of juju models."""
1556+ model_foo = {"name": "admin/foo", "short-name": "foo", "uuid": "129bd0f0"}
1557+ model_bar = {"name": "admin/bar", "short-name": "bar", "uuid": "b513b5e3"}
1558+ model_list = [{"models": [model_foo, model_bar]}] if success else []
1559+
1560+ controller_name = "controller_1"
1561+ controllers = {controller_name: {}} if success else {}
1562+
1563+ run_cmd_mock = mocker.patch.object(cloud.Cloud, "run_command", return_value=success)
1564+ mocker.patch.object(cloud.Cloud, "parse_yaml", return_value=model_list)
1565+ mocker.patch.object(cloud.Cloud, "get_juju_controllers", return_value=controllers)
1566+
1567+ cloud_instance = cloud.Cloud(name="Test cloud")
1568+ cloud_instance.cloud_state = controllers
1569+
1570+ result = cloud_instance.get_juju_models()
1571+
1572+ if success:
1573+ assert result
1574+ run_cmd_mock.assert_called_once_with(
1575+ "juju models -c {} --format yaml".format(controller_name)
1576 )
1577- assert cloud.cloud_state == expected_cloud_state
1578+ for model in [model_foo, model_bar]:
1579+ model_name = model["short-name"]
1580+ model_config = cloud_instance.cloud_state[controller_name]["models"][
1581+ model_name
1582+ ]["config"]
1583+ assert model_config == model
1584+ else:
1585+ assert not result
1586+ for model in [model_foo, model_bar]:
1587+ model_name = model["short-name"]
1588+ assert model_name not in cloud_instance.cloud_state.get(
1589+ controller_name, {}
1590+ ).get("models", {})
1591+
1592+
1593+@pytest.mark.parametrize("success", [True, False])
1594+def test_get_juju_state(cloud_instance, success, mocker):
1595+ """Test function "get_juju_state" that updates local juju state."""
1596+ controller_foo = {
1597+ "models": {"foo_1": "foo_1_model_data", "foo_2": "foo_2_model_data"}
1598+ }
1599+ controller_bar = {
1600+ "models": {"bar_1": "bar_1_model_data", "bar_2": "bar_2_model_data"}
1601+ }
1602+ cloud_state = {"controller_foo": controller_foo, "controller_bar": controller_bar}
1603+
1604+ expected_calls = [
1605+ call("controller_foo", "foo_1"),
1606+ call("controller_foo", "foo_2"),
1607+ call("controller_bar", "bar_1"),
1608+ call("controller_bar", "bar_2"),
1609+ ]
1610+
1611+ mocker.patch.object(cloud_instance, "get_juju_models", return_value=success)
1612+ get_status_mock = mocker.patch.object(cloud_instance, "get_juju_status")
1613+ get_bundle_mock = mocker.patch.object(cloud_instance, "get_juju_bundle")
1614+
1615+ cloud_instance.cloud_state = cloud_state
1616+
1617+ result = cloud_instance.get_juju_state()
1618+
1619+ if success:
1620+ assert result
1621+ get_status_mock.assert_has_calls(expected_calls)
1622+ get_bundle_mock.assert_has_calls(expected_calls)
1623+ else:
1624+ assert not result
1625+ get_status_mock.assert_not_called()
1626+ get_bundle_mock.assert_not_called()
1627+
1628+
1629+def test_get_juju_status(cloud_instance, mocker):
1630+ """Test updating status of a selected model."""
1631+ model_version = "1"
1632+ model_name = "foo model"
1633+ controller_name = "foo controller"
1634+ machine_1_implicit_name = "1"
1635+ machine_2_implicit_name = "2"
1636+ machine_2_explicit_name = "explicit_machine - 2"
1637+ cmd_output_mock = "foo data"
1638+ model_status = {
1639+ "model": {"version": model_version},
1640+ "machines": {
1641+ machine_1_implicit_name: {"arch": "x86"},
1642+ machine_2_implicit_name: {
1643+ "arch": "armv7",
1644+ "display-name": machine_2_explicit_name,
1645+ },
1646+ },
1647+ "applications": {
1648+ "ubuntu": {"application-status": {"current": "ready"}},
1649+ "ntp": {"application-status": {"current": "waiting"}},
1650+ },
1651+ }
1652+
1653+ run_cmd_mock = mocker.patch.object(
1654+ cloud_instance, "run_command", return_value=cmd_output_mock
1655+ )
1656+ mocker.patch.object(cloud_instance, "parse_yaml", return_value=[model_status])
1657+
1658+ cloud_instance.cloud_state = {controller_name: {"models": {model_name: {}}}}
1659+
1660+ cloud_instance.get_juju_status(controller_name, model_name)
1661+
1662+ # assert that correct command was called
1663+ run_cmd_mock.assert_called_with(
1664+ "juju status -m {}:{} --format yaml".format(controller_name, model_name)
1665+ )
1666+
1667+ model_data = cloud_instance.cloud_state[controller_name]["models"][model_name]
1668+ # assert that application data was loaded to the model
1669+ assert model_data["applications"] == model_status["applications"]
1670+ # assert that both implicitly and explicitly named machines are loaded in the model
1671+ expected_machines = {
1672+ machine_1_implicit_name: {"arch": "x86", "machine_id": machine_1_implicit_name},
1673+ machine_2_explicit_name: {
1674+ "arch": "armv7",
1675+ "display-name": machine_2_explicit_name,
1676+ "machine_id": machine_2_implicit_name,
1677+ },
1678+ }
1679+ assert model_data["machines"] == expected_machines
1680+
1681+
1682+def test_refresh(cloud_instance, mocker):
1683+ """Test refresh method."""
1684+ expected_result = True
1685+ get_state_mock = mocker.patch.object(
1686+ cloud_instance, "get_juju_state", return_value=expected_result
1687+ )
1688+
1689+ result = cloud_instance.refresh()
1690+
1691+ get_state_mock.assert_called_once()
1692+ assert result == expected_result
1693+
1694+
1695+def test_audit(patch_cloud_init, mocker):
1696+ """Test execution of Audits for all models in cloud."""
1697+ cloud_name = "Test Cloud"
1698+ lint_rules = "some lint rules"
1699+ override_rules = "some overrides"
1700+ cloud_type = "openstack"
1701+ cloud_state = {
1702+ "controller_foo": {
1703+ "models": {
1704+ "model_foo_1": {"name": "foo_1"},
1705+ "model_foo_2": {"name": "foo_2"},
1706+ }
1707+ },
1708+ "controller_bar": {
1709+ "models": {
1710+ "model_bar_1": {"name": "bar_1"},
1711+ "model_bar_2": {"name": "bar_2"},
1712+ }
1713+ },
1714+ }
1715+ expected_linter_init_calls = []
1716+ expected_do_lint_calls = []
1717+ expected_read_rules_calls = []
1718+ for controller, controller_data in cloud_state.items():
1719+ for model, model_data in controller_data["models"].items():
1720+ expected_linter_init_calls.append(
1721+ call(
1722+ cloud_name,
1723+ lint_rules,
1724+ overrides=override_rules,
1725+ cloud_type=cloud_type,
1726+ controller_name=controller,
1727+ model_name=model,
1728+ )
1729+ )
1730+ expected_do_lint_calls.append(call(model_data))
1731+ expected_read_rules_calls.append(call())
1732+
1733+ linter_object_mock = MagicMock()
1734+
1735+ linter_class_mock = mocker.patch.object(
1736+ cloud, "Linter", return_value=linter_object_mock
1737+ )
1738+
1739+ cloud_instance = cloud.Cloud(
1740+ name=cloud_name,
1741+ lint_rules=lint_rules,
1742+ lint_overrides=override_rules,
1743+ cloud_type=cloud_type,
1744+ )
1745+ cloud_instance.cloud_state = cloud_state
1746+
1747+ cloud_instance.audit()
1748+
1749+ linter_class_mock.assert_has_calls(expected_linter_init_calls)
1750+ linter_object_mock.read_rules.assert_has_calls(expected_read_rules_calls)
1751+ linter_object_mock.do_lint.assert_has_calls(expected_do_lint_calls)
1752diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
1753index 6e40b74..6c611f6 100644
1754--- a/tests/test_jujulint.py
1755+++ b/tests/test_jujulint.py
1756@@ -6,7 +6,7 @@ from unittest import mock
1757
1758 import pytest
1759
1760-from jujulint import check_spaces
1761+from jujulint import check_spaces, lint
1762
1763
1764 class TestUtils:
1765@@ -72,6 +72,31 @@ class TestLinter:
1766 linter.do_lint(juju_status)
1767 assert len(linter.output_collector["errors"]) == 0
1768
1769+ def test_minimal_rules_without_subordinates(self, linter, juju_status):
1770+ """Process rules with none of the applications having subordinate charms."""
1771+ juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"].pop("subordinates")
1772+ juju_status["applications"].pop("ntp")
1773+ linter.lint_rules["subordinates"] = {}
1774+
1775+ linter.do_lint(juju_status)
1776+ assert len(linter.output_collector["errors"]) == 0
1777+
1778+ def test_minimal_rules_json_output(self, linter, juju_status, mocker):
1779+ """Process rules and print output in json format."""
1780+ expected_output = "{result: dict}"
1781+ json_mock = mocker.patch.object(
1782+ lint.json, "dumps", return_value=expected_output
1783+ )
1784+ print_mock = mocker.patch("builtins.print")
1785+
1786+ linter.output_format = "json"
1787+ linter.do_lint(juju_status)
1788+
1789+ json_mock.assert_called_once_with(
1790+ linter.output_collector, indent=2, sort_keys=True
1791+ )
1792+ print_mock.assert_called_once_with(expected_output)
1793+
1794 def test_charm_identification(self, linter, juju_status):
1795 """Test that applications are mapped to charms."""
1796 juju_status["applications"]["ubuntu2"] = {
1797@@ -206,6 +231,19 @@ class TestLinter:
1798 assert errors[0]["id"] == "AZ-invalid-number"
1799 assert errors[0]["num_azs"] == 2
1800
1801+ def test_az_missing(self, linter, juju_status, mocker):
1802+ """Test that AZ parsing logs warning if AZ is not found."""
1803+ # duplicate a AZ name so we have 2 AZs instead of the expected 3
1804+ juju_status["machines"]["2"]["hardware"] = ""
1805+ expected_msg = (
1806+ "Machine 2 has no availability-zone info in hardware field; skipping."
1807+ )
1808+ logger_mock = mocker.patch.object(linter, "_log_with_header")
1809+
1810+ linter.do_lint(juju_status)
1811+
1812+ logger_mock.assert_any_call(expected_msg, level=logging.WARN)
1813+
1814 def test_az_balancing(self, linter, juju_status):
1815 """Test that applications are balanced across AZs."""
1816 # add an extra machine in an existing AZ
1817@@ -614,8 +652,8 @@ applications:
1818 assert errors[0]["expected_value"] == 3
1819 assert errors[0]["actual_value"] == 0
1820
1821- def test_config_isset_false(self, linter, juju_status):
1822- """Test the config condition 'isset' false."""
1823+ def test_config_isset_false_fail(self, linter, juju_status):
1824+ """Test error handling if config condition 'isset'=false is not met."""
1825 linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"isset": False}}}
1826 juju_status["applications"]["ubuntu"]["options"] = {"fake-opt": 0}
1827 linter.do_lint(juju_status)
1828@@ -627,8 +665,17 @@ applications:
1829 assert errors[0]["rule"] == "fake-opt"
1830 assert errors[0]["actual_value"] == 0
1831
1832- def test_config_isset_true(self, linter, juju_status):
1833- """Test the config condition 'isset' true."""
1834+ def test_config_isset_false_pass(self, linter, juju_status):
1835+ """Test handling if config condition 'isset'=false is met."""
1836+ linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"isset": False}}}
1837+ juju_status["applications"]["ubuntu"]["options"] = {}
1838+ linter.do_lint(juju_status)
1839+
1840+ errors = linter.output_collector["errors"]
1841+ assert len(errors) == 0
1842+
1843+ def test_config_isset_true_fail(self, linter, juju_status):
1844+ """Test error handling if config condition 'isset'=true is not met."""
1845 linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"isset": True}}}
1846 juju_status["applications"]["ubuntu"]["options"] = {}
1847 linter.do_lint(juju_status)
1848@@ -639,6 +686,15 @@ applications:
1849 assert errors[0]["application"] == "ubuntu"
1850 assert errors[0]["rule"] == "fake-opt"
1851
1852+ def test_config_isset_true_pass(self, linter, juju_status):
1853+ """Test handling if config condition 'isset'=true is met."""
1854+ linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"isset": True}}}
1855+ juju_status["applications"]["ubuntu"]["options"] = {"fake-opt": 0}
1856+ linter.do_lint(juju_status)
1857+
1858+ errors = linter.output_collector["errors"]
1859+ assert len(errors) == 0
1860+
1861 def test_config_search_valid(self, linter, juju_status):
1862 """Test the config condition 'search' when valid."""
1863 linter.lint_rules["config"] = {
1864@@ -670,6 +726,62 @@ applications:
1865 assert errors[0]["expected_value"] == "\\W\\*, \\W\\*, 25000, 27500"
1866 assert errors[0]["actual_value"] == "[[/, queue1, 10, 20], [\\*, \\*, 10, 20]]"
1867
1868+ def test_config_search_missing(self, linter, mocker):
1869+ """Test the config search method logs warning if the config option is missing."""
1870+ app_name = "ubuntu"
1871+ check_value = 0
1872+ config_key = "missing-opt"
1873+ app_config = {}
1874+ expected_log = (
1875+ "Application {} has no config for '{}', can't search the regex pattern "
1876+ "{}."
1877+ ).format(app_name, config_key, repr(check_value))
1878+
1879+ logger_mock = mocker.patch.object(linter, "_log_with_header")
1880+
1881+ result = linter.search(app_name, check_value, config_key, app_config)
1882+
1883+ assert result is False
1884+ logger_mock.assert_called_once_with(expected_log, level=logging.WARN)
1885+
1886+ def test_check_config_generic_missing_option(self, linter, mocker):
1887+ """Test behavior of check_config_generic() when config option is missing."""
1888+ operator_ = lint.ConfigOperator(
1889+ name="eq", repr="==", check=None, error_template=""
1890+ )
1891+ app_name = "ubuntu"
1892+ check_value = 0
1893+ config_key = "missing-opt"
1894+ app_config = {}
1895+ expected_log = (
1896+ "Application {} has no config for '{}', cannot determine if {} " "{}."
1897+ ).format(app_name, config_key, operator_.repr, repr(check_value))
1898+
1899+ logger_mock = mocker.patch.object(linter, "_log_with_header")
1900+
1901+ result = linter.check_config_generic(
1902+ operator_, app_name, check_value, config_key, app_config
1903+ )
1904+
1905+ logger_mock.assert_called_once_with(expected_log, level=logging.WARN)
1906+ assert result is False
1907+
1908+ def test_check_config_unknown_check_operator(self, linter, mocker):
1909+ """Test that warning is logged when unknown check operator is encountered."""
1910+ app_name = "ubuntu"
1911+ config = {}
1912+ bad_rule = "bad_rule"
1913+ bad_check = "bad_check"
1914+ rules = {bad_rule: {bad_check: 0}}
1915+ expected_log = (
1916+ "Application {} has unknown check operation for {}: " "{}."
1917+ ).format(app_name, bad_rule, bad_check)
1918+
1919+ logger_mock = mocker.patch.object(linter, "_log_with_header")
1920+
1921+ linter.check_config(app_name, config, rules)
1922+ logger_mock.assert_any_call(expected_log, level=logging.WARN)
1923+
1924 def test_parse_cmr_apps_export_bundle(self, linter):
1925 """Test the charm CMR parsing for bundles."""
1926 parsed_yaml = {
1927@@ -703,6 +815,16 @@ applications:
1928 linter.parse_cmr_apps(parsed_yaml)
1929 assert linter.model.cmr_apps == {"grafana", "nagios"}
1930
1931+ def test_parse_cmr_apps_graylog(self, linter):
1932+ """Test the charm CMR parsing adds elasticsearch dependency if graylog is present."""
1933+ parsed_yaml = {
1934+ "saas": {
1935+ "graylog": {"url": "foundations-maas:admin/lma.graylog"},
1936+ }
1937+ }
1938+ linter.parse_cmr_apps(parsed_yaml)
1939+ assert linter.model.cmr_apps == {"graylog", "elasticsearch"}
1940+
1941 def test_check_charms_ops_mandatory_crm_success(self, linter):
1942 """
1943 Test the logic for checking ops mandatory charms provided via CMR.
1944@@ -748,8 +870,9 @@ applications:
1945 rules_path.write_text('---\nkey:\n "value"')
1946
1947 linter.filename = str(rules_path)
1948- linter.read_rules()
1949+ result = linter.read_rules()
1950 assert linter.lint_rules == {"key": "value"}
1951+ assert result
1952
1953 def test_read_rules_include(self, linter, tmp_path):
1954 """Test that rules YAML with an include is imported as expected."""
1955@@ -760,8 +883,41 @@ applications:
1956 rules_path.write_text('---\n!include include.yaml\nkey:\n "value"')
1957
1958 linter.filename = str(rules_path)
1959- linter.read_rules()
1960+ result = linter.read_rules()
1961 assert linter.lint_rules == {"key": "value", "key-inc": "value2"}
1962+ assert result
1963+
1964+ def test_read_rules_overrides(self, linter, tmp_path):
1965+ """Test application of override values to the rules."""
1966+ rules_path = tmp_path / "rules.yaml"
1967+ rules_path.write_text('---\nkey:\n "value"\nsubordinates: {}')
1968+
1969+ linter.overrides = "override_1:value_1#override_2:value_2"
1970+
1971+ linter.filename = str(rules_path)
1972+ linter.read_rules()
1973+ assert linter.lint_rules == {
1974+ "key": "value",
1975+ "subordinates": {
1976+ "override_1": {"where": "value_1"},
1977+ "override_2": {"where": "value_2"},
1978+ },
1979+ }
1980+
1981+ def test_read_rules_fail(self, linter, mocker):
1982+ """Test handling of a read_rules() failure."""
1983+ rule_file = "rules.yaml"
1984+ mocker.patch.object(lint.os.path, "isfile", return_value=False)
1985+ logger_mock = mock.MagicMock()
1986+ linter.logger = logger_mock
1987+ linter.filename = rule_file
1988+
1989+ result = linter.read_rules()
1990+
1991+ assert not result
1992+ logger_mock.error.assert_called_once_with(
1993+ "Rules file {} does not exist.".format(rule_file)
1994+ )
1995
1996 check_spaces_example_bundle = {
1997 "applications": {
1998@@ -993,3 +1149,65 @@ applications:
1999
2000 linter.check_spaces(bundle)
2001 assert logger_mock.warning.call_args_list == expected_warning_callings
2002+
2003+ def test_check_spaces_exception_handling(self, linter, mocker):
2004+ """Test exception handling during check_spaces() method."""
2005+ logger_mock = mock.MagicMock()
2006+ expected_traceback = "python traceback"
2007+ expected_msg = (
2008+ "Exception caught during space check; please check space "
2009+ "by hand. {}".format(expected_traceback)
2010+ )
2011+ mocker.patch.object(linter, "_handle_space_mismatch", side_effect=RuntimeError)
2012+ mocker.patch.object(
2013+ lint.traceback, "format_exc", return_value=expected_traceback
2014+ )
2015+ linter.logger = logger_mock
2016+ linter.model.app_to_charm = self.check_spaces_example_app_charm_map
2017+
2018+ # Run the space check.
2019+ # Based on the above bundle, we should have exactly one mismatch.
2020+ linter.check_spaces(self.check_spaces_example_bundle)
2021+
2022+ logger_mock.warn.assert_called_once_with(expected_msg)
2023+
2024+ @pytest.mark.parametrize(
2025+ "regex_error, check_value, actual_value",
2026+ [
2027+ (True, "same", "same"),
2028+ (True, "same", "different"),
2029+ (False, "same", "same"),
2030+ (False, "same", "different"),
2031+ ],
2032+ )
2033+ def test_helper_operator_check(
2034+ self, regex_error, check_value, actual_value, mocker
2035+ ):
2036+ """Test comparing values using "helper_operator_check()" function."""
2037+ if regex_error:
2038+ mocker.patch.object(lint.re, "match", side_effect=lint.re.error(""))
2039+
2040+ expected_result = check_value == actual_value
2041+
2042+ result = lint.helper_operator_eq_check(check_value, actual_value)
2043+
2044+ assert bool(result) == expected_result
2045+
2046+ @pytest.mark.parametrize(
2047+ "input_str, expected_int",
2048+ [
2049+ (1, 1), # return non-strings unchanged
2050+ ("not_number_1", "not_number_1"), # return non-numbers unchanged
2051+ ("not_number_g", "not_number_g"), # invalid value with valid suffix
2052+ ("2f", "2f"), # unrecognized suffix returns value unchanged
2053+ ("2k", 2000), # convert kilo suffix with quotient 1000
2054+ ("2K", 2048), # convert Kilo suffix with quotient 1024
2055+ ("2m", 2000000), # convert mega suffix with quotient 1000
2056+ ("2M", 2097152), # convert Mega suffix with quotient 1024
2057+ ("2g", 2000000000), # convert giga suffix with quotient 1000
2058+ ("2G", 2147483648), # convert Giga suffix with quotient 1024
2059+ ],
2060+ )
2061+ def test_linter_atoi(self, input_str, expected_int, linter):
2062+ """Test conversion of string values (e.g. 2M (Megabytes)) to integers."""
2063+ assert linter.atoi(input_str) == expected_int
2064diff --git a/tests/test_k8s.py b/tests/test_k8s.py
2065new file mode 100644
2066index 0000000..67835ad
2067--- /dev/null
2068+++ b/tests/test_k8s.py
2069@@ -0,0 +1,29 @@
2070+"""Tests for Kubernetes cloud module."""
2071+from unittest.mock import MagicMock
2072+
2073+from jujulint.k8s import Cloud, Kubernetes
2074+
2075+
2076+def test_init():
2077+ """Test Openstack cloud class initiation."""
2078+ cloud = Kubernetes(name="foo")
2079+ assert cloud.cloud_type == "kubernetes"
2080+
2081+
2082+def test_audit(mocker):
2083+ """Test openstack-specific steps of audit method.
2084+
2085+ Note: Currently this method does not do anything different than its
2086+ parent method.
2087+ """
2088+ audit_mock = mocker.patch.object(Cloud, "audit")
2089+ logger_mock = MagicMock()
2090+
2091+ name = "foo"
2092+ cloud = Kubernetes(name=name)
2093+ cloud.logger = logger_mock
2094+ expected_msg = "[{}] Running Kubernetes-specific audit steps.".format(name)
2095+ cloud.audit()
2096+
2097+ logger_mock.info.assert_called_once_with(expected_msg)
2098+ audit_mock.assert_called_once()
2099diff --git a/tests/test_logging.py b/tests/test_logging.py
2100new file mode 100644
2101index 0000000..ad23ac7
2102--- /dev/null
2103+++ b/tests/test_logging.py
2104@@ -0,0 +1,216 @@
2105+"""Test for jujulint logging module."""
2106+import sys
2107+from unittest.mock import MagicMock, call
2108+
2109+import pytest
2110+
2111+from jujulint import logging
2112+
2113+
2114+def test_logger_init_with_handlers(mocker):
2115+ """Test initiation of a Logger instance with handlers already present."""
2116+ color_logger_mock = mocker.patch.object(logging, "colorlog")
2117+ color_logger_instance_mock = MagicMock()
2118+ color_logger_instance_mock.handlers = ["handler1", "handler2"]
2119+ color_logger_mock.getLogger.return_value = color_logger_instance_mock
2120+ set_level_mock = mocker.patch.object(logging.Logger, "set_level")
2121+
2122+ level = "Debug"
2123+ _ = logging.Logger(level=level)
2124+
2125+ color_logger_mock.getLogger.assert_called_once()
2126+ set_level_mock.assert_called_once_with(level)
2127+ # No new logger handlers were added since the logger instance already had some.
2128+ color_logger_instance_mock.addHandler.assert_not_called()
2129+
2130+
2131+@pytest.mark.parametrize("setup_file_logger", (False, True))
2132+def test_logger_init_without_handlers(setup_file_logger, mocker):
2133+ """Test initiation of a Logger instance that needs to create its own handlers.
2134+
2135+ This test has two variants, with and without setting up a file logger as well.
2136+ """
2137+ logfile = "/tmp/foo" if setup_file_logger else None
2138+ # Mock getLogger and resulting object
2139+ console_logger_mock = MagicMock()
2140+ file_logger_mock = MagicMock()
2141+ get_logger_mock = mocker.patch.object(
2142+ logging.colorlog,
2143+ "getLogger",
2144+ side_effect=[
2145+ console_logger_mock,
2146+ file_logger_mock,
2147+ ],
2148+ )
2149+ console_logger_mock.handlers = []
2150+
2151+ # Mock FileHandler and FileFormatter
2152+ filehandler_mock = MagicMock()
2153+ mocker.patch.object(logging.logging, "FileHandler", return_value=filehandler_mock)
2154+
2155+ file_formatter_mock = MagicMock()
2156+ mocker.patch.object(logging.logging, "Formatter", return_value=file_formatter_mock)
2157+
2158+ # Mock StreamHandler and resulting object
2159+ streamhandler_mock = MagicMock()
2160+ mocker.patch.object(
2161+ logging.colorlog, "StreamHandler", return_value=streamhandler_mock
2162+ )
2163+
2164+ set_level_mock = mocker.patch.object(logging.Logger, "set_level")
2165+ # Mock TTYColorFormatter
2166+ color_formatter_instance = MagicMock()
2167+ color_formatter_mock = mocker.patch.object(
2168+ logging.colorlog, "TTYColoredFormatter", return_value=color_formatter_instance
2169+ )
2170+ level = "Debug"
2171+ logformat_string = "%(log_color)s%(asctime)s [%(levelname)s] %(message)s"
2172+ date_format = "%Y-%m-%d %H:%M:%S"
2173+
2174+ _ = logging.Logger(level=level, logfile=logfile)
2175+
2176+ # Test creation of new log handler
2177+ set_level_mock.assert_called_once_with(level)
2178+ color_formatter_mock.assert_called_once_with(
2179+ logformat_string,
2180+ datefmt=date_format,
2181+ log_colors={
2182+ "DEBUG": "cyan",
2183+ "INFO": "green",
2184+ "WARNING": "yellow",
2185+ "ERROR": "red",
2186+ "CRITICAL": "red,bg_white",
2187+ },
2188+ stream=sys.stdout,
2189+ )
2190+ streamhandler_mock.setFormatter.assert_called_once_with(color_formatter_instance)
2191+
2192+ if not setup_file_logger:
2193+ # getLogger and addHandler are called only once if we are not setting up file
2194+ # logger
2195+ get_logger_mock.assert_called_once()
2196+ console_logger_mock.addHandler.assert_called_once_with(streamhandler_mock)
2197+ else:
2198+ # These steps need to be verified when __init__ sets up file logger as well
2199+ get_logger_mock.assert_has_calls([call(), call("file")])
2200+
2201+ logging.logging.Formatter.assert_called_once_with(
2202+ logformat_string, datefmt=date_format
2203+ )
2204+ assert not file_logger_mock.propagate
2205+ logging.logging.FileHandler.assert_called_once_with(logfile)
2206+ filehandler_mock.setFormatter.assert_called_once_with(file_formatter_mock)
2207+
2208+ console_logger_mock.addHandler.assert_has_calls(
2209+ [
2210+ call(streamhandler_mock),
2211+ call(filehandler_mock),
2212+ ]
2213+ )
2214+ file_logger_mock.addHandler.assert_called_once_with(filehandler_mock)
2215+
2216+
2217+@pytest.mark.parametrize("exit_code", [None, 0, 1, 2])
2218+def test_fubar(exit_code, mocker):
2219+ """Test method that prints to STDERR and ends program execution."""
2220+ err_msg = "foo bar"
2221+ expected_error = "E: {}\n".format(err_msg)
2222+
2223+ mocker.patch.object(logging.sys.stderr, "write")
2224+ mocker.patch.object(logging.sys, "exit")
2225+
2226+ if exit_code is None:
2227+ expected_exit_code = 1
2228+ logging.Logger.fubar(err_msg)
2229+ else:
2230+ expected_exit_code = exit_code
2231+ logging.Logger.fubar(err_msg, exit_code)
2232+
2233+ logging.sys.stderr.write.assert_called_once_with(expected_error)
2234+ logging.sys.exit.assert_called_once_with(expected_exit_code)
2235+
2236+
2237+@pytest.mark.parametrize(
2238+ "loglevel, expected_level",
2239+ [
2240+ ("DEBUG", logging.logging.DEBUG),
2241+ ("INFO", logging.logging.INFO),
2242+ ("WARN", logging.logging.WARN),
2243+ ("ERROR", logging.logging.ERROR),
2244+ ("Foo", logging.logging.INFO),
2245+ ],
2246+)
2247+def test_set_level(loglevel, expected_level, mocker):
2248+ """Test setting various log levels."""
2249+ bound_logger_mock = MagicMock()
2250+ mocker.patch.object(logging.colorlog, "getLogger", return_value=bound_logger_mock)
2251+ basic_config_mock = mocker.patch.object(logging.logging, "basicConfig")
2252+
2253+ logger = logging.Logger()
2254+ logger.set_level(loglevel)
2255+
2256+ if loglevel.lower() == "debug":
2257+ basic_config_mock.assert_called_once_with(level=expected_level)
2258+ else:
2259+ bound_logger_mock.setLevel.assert_called_once_with(expected_level)
2260+
2261+
2262+def test_debug_method(mocker):
2263+ """Test behavior of Logger.debug() method."""
2264+ message = "Log message"
2265+ bound_logger_mock = MagicMock()
2266+ mocker.patch.object(logging.colorlog, "getLogger", return_value=bound_logger_mock)
2267+
2268+ logger = logging.Logger()
2269+ logger.debug(message)
2270+
2271+ bound_logger_mock.debug.assert_called_once_with(message)
2272+
2273+
2274+def test_warn_method(mocker):
2275+ """Test behavior of Logger.warn() method."""
2276+ message = "Log message"
2277+ bound_logger_mock = MagicMock()
2278+ mocker.patch.object(logging.colorlog, "getLogger", return_value=bound_logger_mock)
2279+
2280+ logger = logging.Logger()
2281+ logger.warn(message)
2282+
2283+ bound_logger_mock.warn.assert_called_once_with(message)
2284+
2285+
2286+def test_info_method(mocker):
2287+ """Test behavior of Logger.info() method."""
2288+ message = "Log message"
2289+ bound_logger_mock = MagicMock()
2290+ mocker.patch.object(logging.colorlog, "getLogger", return_value=bound_logger_mock)
2291+
2292+ logger = logging.Logger()
2293+ logger.info(message)
2294+
2295+ bound_logger_mock.info.assert_called_once_with(message)
2296+
2297+
2298+def test_error_method(mocker):
2299+ """Test behavior of Logger.error() method."""
2300+ message = "Log message"
2301+ bound_logger_mock = MagicMock()
2302+ mocker.patch.object(logging.colorlog, "getLogger", return_value=bound_logger_mock)
2303+
2304+ logger = logging.Logger()
2305+ logger.error(message)
2306+
2307+ bound_logger_mock.error.assert_called_once_with(message)
2308+
2309+
2310+def test_log_method(mocker):
2311+ """Test behavior of Logger.log() method."""
2312+ message = "Log message"
2313+ level = logging.logging.INFO
2314+ bound_logger_mock = MagicMock()
2315+ mocker.patch.object(logging.colorlog, "getLogger", return_value=bound_logger_mock)
2316+
2317+ logger = logging.Logger()
2318+ logger.log(message, level)
2319+
2320+ bound_logger_mock.log.assert_called_once_with(level, message)
2321diff --git a/tests/test_openstack.py b/tests/test_openstack.py
2322new file mode 100644
2323index 0000000..cd52b56
2324--- /dev/null
2325+++ b/tests/test_openstack.py
2326@@ -0,0 +1,29 @@
2327+"""Tests for OpenStack cloud module."""
2328+from unittest.mock import MagicMock
2329+
2330+from jujulint.openstack import Cloud, OpenStack
2331+
2332+
2333+def test_init():
2334+ """Test Openstack cloud class initiation."""
2335+ cloud = OpenStack(name="foo")
2336+ assert cloud.cloud_type == "openstack"
2337+
2338+
2339+def test_audit(mocker):
2340+ """Test openstack-specific steps of audit method.
2341+
2342+ Note: Currently this method does not do anything different than its
2343+ parent method.
2344+ """
2345+ audit_mock = mocker.patch.object(Cloud, "audit")
2346+ logger_mock = MagicMock()
2347+
2348+ name = "foo"
2349+ cloud = OpenStack(name=name)
2350+ cloud.logger = logger_mock
2351+ expected_msg = "[{}] Running OpenStack-specific audit steps.".format(name)
2352+ cloud.audit()
2353+
2354+ logger_mock.info.assert_called_once_with(expected_msg)
2355+ audit_mock.assert_called_once()
2356diff --git a/tox.ini b/tox.ini
2357index 40349c8..e37275c 100644
2358--- a/tox.ini
2359+++ b/tox.ini
2360@@ -28,6 +28,7 @@ commands =
2361 --new-first \
2362 --last-failed \
2363 --last-failed-no-failures all \
2364+ --cov-fail-under=100 \
2365 --cov-report=term-missing \
2366 --cov-report=annotate:tests/report/coverage-annotated \
2367 --cov-report=html:tests/report/coverage-html \

Subscribers

People subscribed via source and target branches