Merge ~martin-kalcok/juju-lint:unit-tests into juju-lint:master
- Git
- lp:~martin-kalcok/juju-lint
- unit-tests
- Merge into master
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) |
||||
Related bugs: |
|
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.
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Gabriel Cocenza (gabrielcocenza) wrote : | # |
Thanks for the effort Martin.
I'll review on parts and those comments are regarding the test_check_spaces module
Gabriel Cocenza (gabrielcocenza) wrote : | # |
These comments refers to all other modules with the exception of the logging.py that I'll review tomorrow.
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.
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-
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.
Martin Kalcok (martin-kalcok) : | # |
Gabriel Cocenza (gabrielcocenza) : | # |
Martin Kalcok (martin-kalcok) : | # |
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.
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.
Eric Chen (eric-chen) wrote : | # |
A obvious little mistake
Gabriel Cocenza (gabrielcocenza) wrote : | # |
I think this PR needs to fix some merge conflicts. Other than that, LGTM.
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!
Eric Chen (eric-chen) : | # |
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Change successfully merged at revision 3d7a202f2f005c8
Preview Diff
1 | diff --git a/jujulint/check_spaces.py b/jujulint/check_spaces.py |
2 | index 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): |
16 | diff --git a/jujulint/cli.py b/jujulint/cli.py |
17 | index 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() |
27 | diff --git a/jujulint/cloud.py b/jujulint/cloud.py |
28 | index 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) |
61 | diff --git a/jujulint/k8s.py b/jujulint/k8s.py |
62 | index 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() |
103 | diff --git a/jujulint/lint.py b/jujulint/lint.py |
104 | index 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 | ) |
240 | diff --git a/jujulint/logging.py b/jujulint/logging.py |
241 | index 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) |
287 | diff --git a/jujulint/openstack.py b/jujulint/openstack.py |
288 | index 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 |
311 | diff --git a/tests/conftest.py b/tests/conftest.py |
312 | index 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") |
354 | diff --git a/tests/requirements.txt b/tests/requirements.txt |
355 | index 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 |
374 | diff --git a/tests/test_check_spaces.py b/tests/test_check_spaces.py |
375 | new file mode 100644 |
376 | index 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 | + ) |
727 | diff --git a/tests/test_cli.py b/tests/test_cli.py |
728 | index 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() |
1123 | diff --git a/tests/test_cloud.py b/tests/test_cloud.py |
1124 | index 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) |
1752 | diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py |
1753 | index 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 |
2064 | diff --git a/tests/test_k8s.py b/tests/test_k8s.py |
2065 | new file mode 100644 |
2066 | index 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() |
2099 | diff --git a/tests/test_logging.py b/tests/test_logging.py |
2100 | new file mode 100644 |
2101 | index 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) |
2321 | diff --git a/tests/test_openstack.py b/tests/test_openstack.py |
2322 | new file mode 100644 |
2323 | index 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() |
2356 | diff --git a/tox.ini b/tox.ini |
2357 | index 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 \ |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.