diff --git a/src/tctools/patch_plc/patch_plc_class.py b/src/tctools/patch_plc/patch_plc_class.py index 45467b2..57029de 100644 --- a/src/tctools/patch_plc/patch_plc_class.py +++ b/src/tctools/patch_plc/patch_plc_class.py @@ -1,3 +1,4 @@ +import logging from argparse import RawDescriptionHelpFormatter from collections.abc import Iterable from dataclasses import dataclass, field @@ -180,12 +181,12 @@ def operation_merge( """See help info for `merge`.""" new_sources = FileItems.merge(new_sources.values()) - self.skip_file_duplicates(new_sources, current_sources) - sizes_all = (len(new_sources.folders), len(new_sources.files)) new_sources.subtract(current_sources) + self.skip_file_duplicates(new_sources, current_sources) + self.logger.info( f"Discovered {sizes_all[1]} source files, of which " f"{len(new_sources.files)} are unregistered" @@ -200,6 +201,8 @@ def operation_merge( self.logger.info("No new source files or folders, stopping") return 0 + self.log_sources(new_sources, True) + if self.args.check: self.logger.info("Some file or folders would be added") return 1 # Something left to do, so exit with error (= check has failed) @@ -229,6 +232,8 @@ def operation_remove( self.logger.info("No files or folders to un-register, stopping") return 0 + self.log_sources(to_remove, False) + if self.args.check: self.logger.info("Some file or folders would be un-registered") return 1 # Something left to do, so exit with error (= check has failed) @@ -278,6 +283,9 @@ def operation_reset(self, current_sources: FileItems, new_sources: FileItemsGrou self.logger.info("No files or folders to change, stopping") return 0 + self.log_sources(to_add, True) + self.log_sources(to_remove, False) + if self.args.check: self.logger.info("Some file or folders would be (un-)registered") return 1 # Something left to do, so exit with error (= check has failed) @@ -432,6 +440,20 @@ def remove_helper(ref_elements, ref_set): remove_helper(self._element_folders, to_remove.folders) remove_helper(self._element_files, to_remove.files) + def log_sources(self, source: FileItems, add: bool): + """Log a set of sources in its entirety. + + Logged a INFO level when `dry` or `check` (otherwise the output is kind of + meaningless), otherwise at DEBUG. + """ + level = logging.INFO if self.args.check or self.args.dry else logging.DEBUG + + action = "New" if add else "Remove" + for item in source.files: + self.logger.log(level, f"{action} file: {item}") + for item in source.folders: + self.logger.log(level, f"{action} folder: {item}") + @staticmethod def path_to_str(path: PurePath) -> str: """Turn any path into a windows-path string. diff --git a/tests/plc_code/TwinCAT Project1/MyPlc/POUs/untracked_source/MyPlc_with_untracked.plcproj.xml b/tests/plc_code/TwinCAT Project1/MyPlc/MyPlc_with_untracked.plcproj.xml similarity index 100% rename from tests/plc_code/TwinCAT Project1/MyPlc/POUs/untracked_source/MyPlc_with_untracked.plcproj.xml rename to tests/plc_code/TwinCAT Project1/MyPlc/MyPlc_with_untracked.plcproj.xml diff --git a/tests/test_patch_plc.py b/tests/test_patch_plc.py index 625d4a2..1dbec9b 100644 --- a/tests/test_patch_plc.py +++ b/tests/test_patch_plc.py @@ -2,6 +2,7 @@ import shutil import subprocess import sys +from collections.abc import Generator from pathlib import Path, PureWindowsPath import pytest @@ -21,6 +22,16 @@ def to_paths(*paths: str) -> list[Path]: return [Path(p) for p in paths] +@pytest.fixture() +def plc_dir(plc_code) -> Generator[Path, None, None]: + yield plc_code / "TwinCAT Project1" / "MyPlc" + + +@pytest.fixture() +def project(plc_dir) -> Generator[Path, None, None]: + yield plc_dir / "MyPlc.plcproj" + + def test_help(capsys): """Test the help text.""" with pytest.raises(SystemExit) as err: @@ -32,12 +43,12 @@ def test_help(capsys): assert "usage:" in message -def test_cli(plc_code): +def test_cli(plc_dir, project): """Test the CLI hook works.""" - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" source = plc_dir / "POUs" / "untracked_source" + assert "untracked_source" not in project.read_text() + path = sys.executable # Re-use whatever executable we're using now result = subprocess.run( [path, "-m", "tctools.patch_plc", str(project), "merge", "-r", str(source)], @@ -45,14 +56,12 @@ def test_cli(plc_code): ) assert result.returncode == 0 - # assert "Re-saved 1 path" in result.stdout.decode() - # TODO: Add some kind of output assertion here + assert "untracked_source" in project.read_text() + # More detailed assertions we will perform in the next tests -def test_merge_single_file(plc_code): +def test_merge_single_file(plc_dir, project): """Test happy-flow adding.""" - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" source = plc_dir / "POUs" / "untracked_source" / "F_UntrackedFunc.TcPOU" patcher = PatchPlc(str(project), "merge", str(source)) @@ -70,11 +79,13 @@ def test_merge_single_file(plc_code): assert '' in project_content -def test_merge_recursive(plc_code): - """Test happy-flow adding for a complete folder.""" - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" - source = plc_dir / "POUs" / "untracked_source" +@pytest.mark.parametrize("target", ["POUs/untracked_source/", "./"]) +def test_merge_recursive(plc_dir, project, target, caplog): + """Test happy-flow adding for a complete folder. + + Try both on a completely new folder and the entire project root + """ + source = plc_dir / Path(target) untracked_files = to_paths( "POUs/untracked_source/F_UntrackedFunc.TcPOU", @@ -92,8 +103,8 @@ def test_merge_recursive(plc_code): for path in untracked_files + untracked_folders: assert path_to_str(path) not in project_content - patcher = PatchPlc(str(project), "merge", str(source), "-r") - patcher.run() + with caplog.at_level(logging.WARNING): + PatchPlc(str(project), "merge", str(source), "-r").run() project_content = project.read_text() @@ -102,19 +113,18 @@ def test_merge_recursive(plc_code): for folder in untracked_folders: assert f'' in project_content + assert len(caplog.records) == 0 # No warnings or errors + # Now compare with stored, sorted result: - expected_file = plc_code / source / "MyPlc_with_untracked.plcproj.xml" + expected_file = plc_dir / "MyPlc_with_untracked.plcproj.xml" XmlSorter(str(project)).run() assert project.read_text() == expected_file.read_text() -def test_remove(plc_code): +def test_remove(plc_dir, project): """Test remove happy-flow.""" - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" - tracked_files = to_paths( "POUs/FB_Example.TcPOU", "DUTs/ST_Example.TcDUT", @@ -136,14 +146,11 @@ def test_remove(plc_code): assert path_to_str(file) not in project_content lines_after = project_content.count("\n") - assert lines_before - 6 == lines_after # Make sure not more got deleted + assert lines_after == lines_before - 6 # Make sure not more got deleted @pytest.mark.parametrize("recursive", [False, True]) -def test_remove_recursive(plc_code, recursive): - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" - +def test_remove_recursive(plc_dir, project, recursive): folder = Path("POUs/Module") tracked_folders = [ 'Include="POUs\\Module"', @@ -173,19 +180,16 @@ def test_remove_recursive(plc_code, recursive): lines_after = content_after.count("\n") if not recursive: - assert content_before == content_after # No changes + assert content_after == content_before # No changes return for item in tracked_files + tracked_folders: assert item not in content_after - assert lines_before - 2 * 1 - 3 * 3 == lines_after - + assert lines_after == lines_before - 2 * 1 - 3 * 3 -def test_reset(plc_code): - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" +def test_reset(plc_dir, project): # Remove the actual (tracked) folder first: shutil.rmtree(plc_dir / "POUs" / "Module") # And there is already the "POUs/untracked_source" directory @@ -225,14 +229,11 @@ def test_reset(plc_code): assert item in content_after lines_after = content_after.count("\n") - assert lines_before - (2 * 1) - (3 * 3) + (2 * 1) + (4 * 3) == lines_after + assert lines_after == lines_before - (2 * 1) - (3 * 3) + (2 * 1) + (4 * 3) # Remove 2 folders and 3 files, add 2 folders and 4 files -def test_reset_duplicate(plc_code, caplog): - plc_dir = plc_code / "TwinCAT Project1" / "MyPlc" - project = plc_dir / "MyPlc.plcproj" - +def test_reset_duplicate(plc_dir, project, caplog): # Create a new file, with a name that's already known: module_folder = plc_dir / "POUs" / "Module" new_file = module_folder / "DUTs" / "MAIN.TcPOU" @@ -249,4 +250,4 @@ def test_reset_duplicate(plc_code, caplog): msg = caplog.records[0].message assert "Refusing to add" in msg and "MAIN.TcPOU" in msg - assert content_before == project.read_text() # The project should not be changed + assert project.read_text() == content_before # The project should not be changed