From 5ca56796fd66905fbdecf9ea234e247a5691090c Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Mon, 9 Oct 2023 12:50:01 +0100 Subject: [PATCH] Simplify transformer testing (#3820) --- .gitignore | 1 + src/ansiblelint/file_utils.py | 10 ++++-- .../rules/deprecated_local_action.py | 33 ++++++++--------- src/ansiblelint/rules/jinja.py | 29 ++++++++------- src/ansiblelint/rules/no_log_password.py | 32 +++++++++-------- src/ansiblelint/testing/fixtures.py | 22 ------------ test/test_transformer.py | 36 +++++++++---------- 7 files changed, 77 insertions(+), 86 deletions(-) diff --git a/.gitignore b/.gitignore index d120b79530..9c7794378f 100644 --- a/.gitignore +++ b/.gitignore @@ -72,3 +72,4 @@ test/schemas/node_modules collections site _readthedocs +*.tmp.* diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index 4452d029e9..5a18870c1b 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -371,10 +371,16 @@ def write(self, *, force: bool = False) -> None: lintable.write(force=True) """ - if not force and not self.updated: + dump_filename = self.path.expanduser().resolve() + if os.environ.get("ANSIBLE_LINT_WRITE_TMP", "0") == "1": + dump_filename = dump_filename.with_suffix( + f".tmp{dump_filename.suffix}", + ) + elif not force and not self.updated: # No changes to write. return - self.path.expanduser().resolve().write_text( + + dump_filename.write_text( self._content or "", encoding="utf-8", ) diff --git a/src/ansiblelint/rules/deprecated_local_action.py b/src/ansiblelint/rules/deprecated_local_action.py index c1a77b0b2c..088ab79e8d 100644 --- a/src/ansiblelint/rules/deprecated_local_action.py +++ b/src/ansiblelint/rules/deprecated_local_action.py @@ -5,7 +5,9 @@ import copy import logging +import os import sys +from pathlib import Path from typing import TYPE_CHECKING from ansiblelint.rules import AnsibleLintRule, TransformMixin @@ -13,8 +15,6 @@ from ansiblelint.transformer import Transformer if TYPE_CHECKING: - from pathlib import Path - from ruamel.yaml.comments import CommentedMap, CommentedSeq from ansiblelint.config import Options @@ -84,6 +84,8 @@ def transform( # testing code to be loaded only with pytest or when executed the rule file if "pytest" in sys.modules: + from unittest import mock + from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner @@ -97,34 +99,33 @@ def test_local_action(default_rules_collection: RulesCollection) -> None: assert len(results) == 1 assert results[0].tag == "deprecated-local-action" + @mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True) def test_local_action_transform( config_options: Options, - copy_examples_dir: tuple[Path, Path], default_rules_collection: RulesCollection, ) -> None: """Test transform functionality for no-log-password rule.""" - playbook: str = "examples/playbooks/tasks/local_action.yml" + playbook = Path("examples/playbooks/tasks/local_action.yml") config_options.write_list = ["all"] - config_options.lintables = [playbook] + config_options.lintables = [str(playbook)] runner_result = get_matches( rules=default_rules_collection, options=config_options, ) transformer = Transformer(result=runner_result, options=config_options) transformer.run() - matches = runner_result.matches assert len(matches) == 3 - orig_dir, tmp_dir = copy_examples_dir - orig_playbook = orig_dir / playbook - expected_playbook = orig_dir / playbook.replace(".yml", ".transformed.yml") - transformed_playbook = tmp_dir / playbook - - orig_playbook_content = orig_playbook.read_text() - expected_playbook_content = expected_playbook.read_text() - transformed_playbook_content = transformed_playbook.read_text() + orig_content = playbook.read_text(encoding="utf-8") + expected_content = playbook.with_suffix( + f".transformed{playbook.suffix}", + ).read_text(encoding="utf-8") + transformed_content = playbook.with_suffix(f".tmp{playbook.suffix}").read_text( + encoding="utf-8", + ) - assert orig_playbook_content != transformed_playbook_content - assert transformed_playbook_content == expected_playbook_content + assert orig_content != transformed_content + assert expected_content == transformed_content + playbook.with_suffix(f".tmp{playbook.suffix}").unlink() diff --git a/src/ansiblelint/rules/jinja.py b/src/ansiblelint/rules/jinja.py index a00eaea17f..9af2d78695 100644 --- a/src/ansiblelint/rules/jinja.py +++ b/src/ansiblelint/rules/jinja.py @@ -2,6 +2,7 @@ from __future__ import annotations import logging +import os import re import sys from dataclasses import dataclass @@ -477,6 +478,8 @@ def blacken(text: str) -> str: if "pytest" in sys.modules: + from unittest import mock + import pytest # pylint: disable=ungrouped-imports @@ -824,16 +827,16 @@ def test_jinja_valid() -> None: errs = Runner(success, rules=collection).run() assert len(errs) == 0 + @mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True) def test_jinja_transform( config_options: Options, - copy_examples_dir: tuple[Path, Path], default_rules_collection: RulesCollection, ) -> None: """Test transform functionality for jinja rule.""" - playbook: str = "examples/playbooks/rule-jinja-before.yml" + playbook = Path("examples/playbooks/rule-jinja-before.yml") config_options.write_list = ["all"] - config_options.lintables = [playbook] + config_options.lintables = [str(playbook)] runner_result = get_matches( rules=default_rules_collection, options=config_options, @@ -844,17 +847,17 @@ def test_jinja_transform( matches = runner_result.matches assert len(matches) == 2 - orig_dir, tmp_dir = copy_examples_dir - orig_playbook = orig_dir / playbook - expected_playbook = orig_dir / playbook.replace(".yml", ".transformed.yml") - transformed_playbook = tmp_dir / playbook - - orig_playbook_content = orig_playbook.read_text() - expected_playbook_content = expected_playbook.read_text() - transformed_playbook_content = transformed_playbook.read_text() + orig_content = playbook.read_text(encoding="utf-8") + expected_content = playbook.with_suffix( + f".transformed{playbook.suffix}", + ).read_text(encoding="utf-8") + transformed_content = playbook.with_suffix(f".tmp{playbook.suffix}").read_text( + encoding="utf-8", + ) - assert orig_playbook_content != transformed_playbook_content - assert transformed_playbook_content == expected_playbook_content + assert orig_content != transformed_content + assert expected_content == transformed_content + playbook.with_suffix(f".tmp{playbook.suffix}").unlink() def _get_error_line(task: dict[str, Any], path: list[str | int]) -> int: diff --git a/src/ansiblelint/rules/no_log_password.py b/src/ansiblelint/rules/no_log_password.py index 5156dad7c7..c3f6d34286 100644 --- a/src/ansiblelint/rules/no_log_password.py +++ b/src/ansiblelint/rules/no_log_password.py @@ -15,7 +15,9 @@ """NoLogPasswordsRule used with ansible-lint.""" from __future__ import annotations +import os import sys +from pathlib import Path from typing import TYPE_CHECKING from ansiblelint.rules import AnsibleLintRule, RulesCollection, TransformMixin @@ -24,8 +26,6 @@ from ansiblelint.utils import Task, convert_to_boolean if TYPE_CHECKING: - from pathlib import Path - from ruamel.yaml.comments import CommentedMap, CommentedSeq from ansiblelint.config import Options @@ -94,6 +94,8 @@ def transform( if "pytest" in sys.modules: + from unittest import mock + import pytest if TYPE_CHECKING: @@ -325,17 +327,17 @@ def test_password_lock_false(rule_runner: RunFromText) -> None: results = rule_runner.run_playbook(PASSWORD_LOCK_FALSE) assert len(results) == 0 + @mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True) def test_no_log_password_transform( config_options: Options, - copy_examples_dir: tuple[Path, Path], ) -> None: """Test transform functionality for no-log-password rule.""" - playbook: str = "examples/playbooks/transform-no-log-password.yml" + playbook = Path("examples/playbooks/transform-no-log-password.yml") config_options.write_list = ["all"] rules = RulesCollection(options=config_options) rules.register(NoLogPasswordsRule()) - config_options.lintables = [playbook] + config_options.lintables = [str(playbook)] runner_result = get_matches(rules=rules, options=config_options) transformer = Transformer(result=runner_result, options=config_options) transformer.run() @@ -343,14 +345,14 @@ def test_no_log_password_transform( matches = runner_result.matches assert len(matches) == 2 - orig_dir, tmp_dir = copy_examples_dir - orig_playbook = orig_dir / playbook - expected_playbook = orig_dir / playbook.replace(".yml", ".transformed.yml") - transformed_playbook = tmp_dir / playbook - - orig_playbook_content = orig_playbook.read_text() - expected_playbook_content = expected_playbook.read_text() - transformed_playbook_content = transformed_playbook.read_text() + orig_content = playbook.read_text(encoding="utf-8") + expected_content = playbook.with_suffix( + f".transformed{playbook.suffix}", + ).read_text(encoding="utf-8") + transformed_content = playbook.with_suffix(f".tmp{playbook.suffix}").read_text( + encoding="utf-8", + ) - assert orig_playbook_content != transformed_playbook_content - assert transformed_playbook_content == expected_playbook_content + assert orig_content != transformed_content + assert expected_content == transformed_content + playbook.with_suffix(f".tmp{playbook.suffix}").unlink() diff --git a/src/ansiblelint/testing/fixtures.py b/src/ansiblelint/testing/fixtures.py index 415e3ce889..fd4cf36319 100644 --- a/src/ansiblelint/testing/fixtures.py +++ b/src/ansiblelint/testing/fixtures.py @@ -8,9 +8,6 @@ from __future__ import annotations import copy -import os -import shutil -from pathlib import Path from typing import TYPE_CHECKING import pytest @@ -21,7 +18,6 @@ from ansiblelint.testing import RunFromText if TYPE_CHECKING: - from argparse import Namespace from collections.abc import Iterator from _pytest.fixtures import SubRequest @@ -65,21 +61,3 @@ def fixture_config_options() -> Iterator[Options]: original_options = copy.deepcopy(options) yield options options = original_options - - -@pytest.fixture(name="copy_examples_dir") -def fixture_copy_examples_dir( - tmp_path: Path, - config_options: Namespace, -) -> Iterator[tuple[Path, Path]]: - """Fixture that copies the examples/ dir into a tmpdir.""" - examples_dir = Path("examples") - - shutil.copytree(examples_dir, tmp_path / "examples") - old_cwd = Path.cwd() - try: - os.chdir(tmp_path) - config_options.cwd = tmp_path - yield old_cwd, tmp_path - finally: - os.chdir(old_cwd) diff --git a/test/test_transformer.py b/test/test_transformer.py index f06302484f..dad039d6bc 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -1,9 +1,11 @@ """Tests for Transformer.""" from __future__ import annotations +import os import shutil from pathlib import Path from typing import TYPE_CHECKING +from unittest import mock import pytest @@ -23,16 +25,16 @@ def fixture_runner_result( config_options: Options, default_rules_collection: RulesCollection, - playbook: str, + playbook_str: str, ) -> LintResult: """Fixture that runs the Runner to populate a LintResult for a given file.""" - config_options.lintables = [playbook] + config_options.lintables = [playbook_str] result = get_matches(rules=default_rules_collection, options=config_options) return result @pytest.mark.parametrize( - ("playbook", "matches_count", "transformed"), + ("playbook_str", "matches_count", "transformed"), ( # reuse TestRunner::test_runner test cases to ensure transformer does not mangle matches pytest.param( @@ -127,10 +129,10 @@ def fixture_runner_result( ), ), ) +@mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True) def test_transformer( # pylint: disable=too-many-arguments, too-many-locals config_options: Options, - copy_examples_dir: tuple[Path, Path], - playbook: str, + playbook_str: str, runner_result: LintResult, transformed: bool, matches_count: int, @@ -139,6 +141,7 @@ def test_transformer( # pylint: disable=too-many-arguments, too-many-locals Based on TestRunner::test_runner """ + playbook = Path(playbook_str) config_options.write_list = ["all"] transformer = Transformer(result=runner_result, options=config_options) transformer.run() @@ -146,21 +149,18 @@ def test_transformer( # pylint: disable=too-many-arguments, too-many-locals matches = runner_result.matches assert len(matches) == matches_count - orig_dir, tmp_dir = copy_examples_dir - orig_playbook = orig_dir / playbook - expected_playbook = orig_dir / playbook.replace(".yml", ".transformed.yml") - transformed_playbook = tmp_dir / playbook - - orig_playbook_content = orig_playbook.read_text() - expected_playbook_content = expected_playbook.read_text() - transformed_playbook_content = transformed_playbook.read_text() - + orig_content = playbook.read_text(encoding="utf-8") if transformed: - assert orig_playbook_content != transformed_playbook_content - else: - assert orig_playbook_content == transformed_playbook_content + expected_content = playbook.with_suffix( + f".transformed{playbook.suffix}", + ).read_text(encoding="utf-8") + transformed_content = playbook.with_suffix(f".tmp{playbook.suffix}").read_text( + encoding="utf-8", + ) - assert transformed_playbook_content == expected_playbook_content + assert orig_content != transformed_content + assert expected_content == transformed_content + playbook.with_suffix(f".tmp{playbook.suffix}").unlink() @pytest.mark.parametrize(