From bf291e19282c8dfc355ec4f01f09880fa51e8af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Thu, 22 Feb 2024 15:30:25 +0100 Subject: [PATCH 01/19] Add: Allow to create a binary file with temp_file context manager Extend temp_file context manager to support also creating binary files. --- pontos/testing/__init__.py | 17 ++++++++++++++--- tests/testing/test_testing.py | 9 +++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/pontos/testing/__init__.py b/pontos/testing/__init__.py index 398de367c..96ad0c836 100644 --- a/pontos/testing/__init__.py +++ b/pontos/testing/__init__.py @@ -11,7 +11,15 @@ import tempfile from contextlib import contextmanager from pathlib import Path -from typing import Any, AsyncIterator, Awaitable, Generator, Iterable, Optional +from typing import ( + Any, + AsyncIterator, + Awaitable, + Generator, + Iterable, + Optional, + Union, +) from pontos.git._git import exec_git from pontos.helper import add_sys_path, ensure_unload_module, unload_module @@ -136,7 +144,7 @@ def temp_git_repository( @contextmanager def temp_file( - content: Optional[str] = None, + content: Optional[Union[str, bytes]] = None, *, name: str = "test.toml", change_into: bool = False, @@ -166,7 +174,10 @@ def temp_file( with temp_directory(change_into=change_into) as tmp_dir: test_file = tmp_dir / name if content: - test_file.write_text(content, encoding="utf8") + if isinstance(content, bytes): + test_file.write_bytes(content) + else: + test_file.write_text(content, encoding="utf8") else: test_file.touch() diff --git a/tests/testing/test_testing.py b/tests/testing/test_testing.py index 6f618ab5d..3e1b68714 100644 --- a/tests/testing/test_testing.py +++ b/tests/testing/test_testing.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later # +import struct import unittest from pathlib import Path @@ -109,6 +110,14 @@ def test_temp_file(self): self.assertFalse(test_file.exists()) + def test_temp_binary_file(self): + data = struct.pack(">if", 42, 2.71828182846) + with temp_file(data) as test_file: + self.assertTrue(test_file.exists()) + self.assertEqual(data, test_file.read_bytes()) + + self.assertFalse(test_file.exists()) + def test_temp_file_without_content(self): with temp_file(name="foo.bar") as test_file: self.assertTrue(test_file.exists()) From e6b5161c294f6c2fe982b4bac8f2c7e51973f472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Thu, 22 Feb 2024 15:32:10 +0100 Subject: [PATCH 02/19] Change: Split and improve update header test cases Split the tests into separate test cases for each function and use context managers for creating temporary directories and files wherever possible. This avoids leaked files if a test fails. --- tests/updateheader/test_header.py | 369 +++++++++++++++--------------- 1 file changed, 187 insertions(+), 182 deletions(-) diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index dbbf57a45..b2ce1baab 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -15,7 +15,7 @@ from unittest.mock import patch from pontos.terminal.terminal import ConsoleTerminal -from pontos.testing import temp_file +from pontos.testing import temp_directory, temp_file from pontos.updateheader.updateheader import ( OLD_COMPANY, _compile_copyright_regex, @@ -36,63 +36,63 @@ from pontos.updateheader.updateheader import ( _remove_outdated_lines as remove_outdated_lines, ) -from pontos.updateheader.updateheader import _update_file as update_file +from pontos.updateheader.updateheader import ( + _update_file as update_file, +) HEADER = """# SPDX-FileCopyrightText: {date} Greenbone AG # # SPDX-License-Identifier: AGPL-3.0-or-later""" +_here_ = Path(__file__).parent + + class Terminal(ConsoleTerminal): @staticmethod def get_width() -> int: return 999 -class UpdateHeaderTestCase(TestCase): - def setUp(self): - self.args = Namespace() - self.args.company = "Greenbone AG" - - self.path = Path(__file__).parent - - self.regex = re.compile( - "(SPDX-FileCopyrightText:|[Cc]opyright).*?(19[0-9]{2}|20[0-9]{2}) " - f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.args.company})" - ) - +class GetModifiedYearTestCase(TestCase): @patch("pontos.updateheader.updateheader.run") def test_get_modified_year(self, run_mock): - test_file = self.path / "test.py" - test_file.touch(exist_ok=True) - - run_mock.return_value = CompletedProcess( - args=[ - "git", - "log", - "-1", - "--format=%ad", - "--date=format:%Y", - f"{test_file}", - ], - returncode=0, - stdout="2020\n", - stderr="", - ) - - year = get_modified_year(f=test_file) - self.assertEqual(year, "2020") + with temp_file(name="test.py", change_into=True) as test_file: + run_mock.return_value = CompletedProcess( + args=[ + "git", + "log", + "-1", + "--format=%ad", + "--date=format:%Y", + f"{test_file}", + ], + returncode=0, + stdout="2020\n", + stderr="", + ) - test_file.unlink() + year = get_modified_year(f=test_file) + self.assertEqual(year, "2020") def test_get_modified_year_error(self): - test_file = self.path / "test.py" - if test_file.exists(): - test_file.unlink() + with ( + temp_directory(change_into=True) as temp_dir, + self.assertRaises(CalledProcessError), + ): + test_file = temp_dir / "test.py" - with self.assertRaises(CalledProcessError): get_modified_year(f=test_file) + +class FindCopyRightTestCase(TestCase): + def setUp(self): + self.company = "Greenbone AG" + self.regex = re.compile( + "(SPDX-FileCopyrightText:|[Cc]opyright).*?(19[0-9]{2}|20[0-9]{2}) " + f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.company})" + ) + def test_find_copyright(self): test_line = "# Copyright (C) 1995-2021 Greenbone AG" test2_line = "# Copyright (C) 1995 Greenbone AG" @@ -109,7 +109,7 @@ def test_find_copyright(self): self.assertIsNotNone(match) self.assertEqual(match["creation_year"], "1995") self.assertEqual(match["modification_year"], "2021") - self.assertEqual(match["company"], self.args.company) + self.assertEqual(match["company"], self.company) # No modification Date found, match = find_copyright( @@ -119,7 +119,7 @@ def test_find_copyright(self): self.assertIsNotNone(match) self.assertEqual(match["creation_year"], "1995") self.assertEqual(match["modification_year"], None) - self.assertEqual(match["company"], self.args.company) + self.assertEqual(match["company"], self.company) # No match found, match = find_copyright( @@ -144,7 +144,7 @@ def test_find_spdx_copyright(self): self.assertIsNotNone(match) self.assertEqual(match["creation_year"], "1995") self.assertEqual(match["modification_year"], "2021") - self.assertEqual(match["company"], self.args.company) + self.assertEqual(match["company"], self.company) # No modification Date found, match = find_copyright( @@ -154,7 +154,7 @@ def test_find_spdx_copyright(self): self.assertIsNotNone(match) self.assertEqual(match["creation_year"], "1995") self.assertEqual(match["modification_year"], None) - self.assertEqual(match["company"], self.args.company) + self.assertEqual(match["company"], self.company) # No match found, match = find_copyright( @@ -163,13 +163,18 @@ def test_find_spdx_copyright(self): self.assertFalse(found) self.assertIsNone(match) + +class AddHeaderTestCase(TestCase): + def setUp(self): + self.company = "Greenbone AG" + def test_add_header(self): expected_header = HEADER.format(date="2021") + "\n" header = add_header( suffix=".py", license_id="AGPL-3.0-or-later", - company=self.args.company, + company=self.company, year="2021", ) @@ -180,7 +185,7 @@ def test_add_header_wrong_file_suffix(self): add_header( suffix=".prr", license_id="AGPL-3.0-or-later", - company=self.args.company, + company=self.company, year="2021", ) @@ -189,10 +194,23 @@ def test_add_header_license_not_found(self): add_header( suffix=".py", license_id="AAAGPL-3.0-or-later", - company=self.args.company, + company=self.company, year="2021", ) + +class UpdateFileTestCase(TestCase): + def setUp(self): + self.args = Namespace() + self.args.company = "Greenbone AG" + + self.path = Path(__file__).parent + + self.regex = re.compile( + "(SPDX-FileCopyrightText:|[Cc]opyright).*?(19[0-9]{2}|20[0-9]{2}) " + f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.args.company})" + ) + @patch("sys.stdout", new_callable=StringIO) def test_update_file_not_existing(self, mock_stdout): self.args.year = "2020" @@ -201,24 +219,23 @@ def test_update_file_not_existing(self, mock_stdout): term = Terminal() - test_file = self.path / "test.py" - if test_file.exists(): - test_file.unlink() - - with self.assertRaises(FileNotFoundError): - update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, + with temp_directory(change_into=True) as temp_dir: + test_file = temp_dir / "test.py" + + with self.assertRaises(FileNotFoundError): + update_file( + file=test_file, + copyright_regex=self.regex, + parsed_args=self.args, + term=term, + ) + + ret = mock_stdout.getvalue() + self.assertEqual( + ret, + f"{test_file}: File is not existing.\n", ) - ret = mock_stdout.getvalue() - self.assertEqual( - ret, - f"{test_file}: File is not existing.\n", - ) - @patch("sys.stdout", new_callable=StringIO) def test_update_file_wrong_license(self, mock_stdout): self.args.year = "2020" @@ -227,25 +244,21 @@ def test_update_file_wrong_license(self, mock_stdout): term = Terminal() - test_file = self.path / "test.py" - test_file.touch() - - code = update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, - ) - self.assertEqual(code, 1) - - ret = mock_stdout.getvalue() - self.assertEqual( - ret, - f"{test_file}: License file for " - "AAAGPL-3.0-or-later is not existing.\n", - ) + with temp_file(name="test.py", change_into=True) as test_file: + code = update_file( + file=test_file, + copyright_regex=self.regex, + parsed_args=self.args, + term=term, + ) + self.assertEqual(code, 1) - test_file.unlink() + ret = mock_stdout.getvalue() + self.assertEqual( + ret, + f"{test_file}: License file for " + "AAAGPL-3.0-or-later is not existing.\n", + ) @patch("sys.stdout", new_callable=StringIO) def test_update_file_suffix_invalid(self, mock_stdout): @@ -255,24 +268,20 @@ def test_update_file_suffix_invalid(self, mock_stdout): term = Terminal() - test_file = self.path / "test.pppy" - test_file.touch() - - code = update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, - ) - self.assertEqual(code, 1) - - ret = mock_stdout.getvalue() - self.assertEqual( - ret, - f"{test_file}: No license header for the format .pppy found.\n", - ) + with temp_file(name="test.pppy", change_into=True) as test_file: + code = update_file( + file=test_file, + copyright_regex=self.regex, + parsed_args=self.args, + term=term, + ) + self.assertEqual(code, 1) - test_file.unlink() + ret = mock_stdout.getvalue() + self.assertEqual( + ret, + f"{test_file}: No license header for the format .pppy found.\n", + ) @patch("sys.stdout", new_callable=StringIO) def test_update_file_binary_file(self, mock_stdout): @@ -282,16 +291,14 @@ def test_update_file_binary_file(self, mock_stdout): term = Terminal() - test_file = self.path / "test.py" - if test_file.exists(): - test_file.unlink() - # create a Binary file ... # https://stackoverflow.com/a/30148554 - with open(test_file, "wb") as f: - f.write(struct.pack(">if", 42, 2.71828182846)) + data = struct.pack(">if", 42, 2.71828182846) - with self.assertRaises(UnicodeDecodeError): + with ( + temp_file(name="test.py", content=data) as test_file, + self.assertRaises(UnicodeDecodeError), + ): code = update_file( file=test_file, copyright_regex=self.regex, @@ -306,8 +313,6 @@ def test_update_file_binary_file(self, mock_stdout): f"{test_file}: Ignoring binary file.\n", ) - test_file.unlink() - @patch("sys.stdout", new_callable=StringIO) def test_update_file_changed(self, mock_stdout): self.args.year = "1995" @@ -316,11 +321,12 @@ def test_update_file_changed(self, mock_stdout): term = Terminal() - test_file = self.path / "test.py" - if test_file.exists(): - test_file.unlink() + with ( + temp_directory(change_into=True) as temp_dir, + self.assertRaises(FileNotFoundError), + ): + test_file = temp_dir / "test.py" - with self.assertRaises(FileNotFoundError): code = update_file( file=test_file, copyright_regex=self.regex, @@ -329,13 +335,13 @@ def test_update_file_changed(self, mock_stdout): ) self.assertEqual(code, 1) - ret = mock_stdout.getvalue() + ret = mock_stdout.getvalue() - self.assertIn(f"{test_file}", ret) - self.assertIn("Could not get date", ret) - self.assertIn("of last modification using git,", ret) - self.assertIn(f"using {self.args.year} instead.", ret) - self.assertIn("File is not existing.", ret) + self.assertIn(f"{test_file}", ret) + self.assertIn("Could not get date", ret) + self.assertIn("of last modification using git,", ret) + self.assertIn(f"using {self.args.year} instead.", ret) + self.assertIn("File is not existing.", ret) @patch("sys.stdout", new_callable=StringIO) def test_update_create_header(self, mock_stdout): @@ -347,24 +353,23 @@ def test_update_create_header(self, mock_stdout): expected_header = HEADER.format(date="1995") + "\n\n" - test_file = self.path / "test.py" - test_file.touch() - - code = update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, - ) + with temp_file(name="test.py", change_into=True) as test_file: + code = update_file( + file=test_file, + copyright_regex=self.regex, + parsed_args=self.args, + term=term, + ) - ret = mock_stdout.getvalue() - self.assertEqual( - f"{test_file}: Added license header.\n", - ret, - ) - self.assertEqual(code, 0) - self.assertEqual(expected_header, test_file.read_text(encoding="utf-8")) - test_file.unlink() + ret = mock_stdout.getvalue() + self.assertEqual( + f"{test_file}: Added license header.\n", + ret, + ) + self.assertEqual(code, 0) + self.assertEqual( + expected_header, test_file.read_text(encoding="utf-8") + ) @patch("sys.stdout", new_callable=StringIO) def test_update_header_in_file(self, mock_stdout): @@ -375,33 +380,27 @@ def test_update_header_in_file(self, mock_stdout): term = Terminal() header = HEADER.format(date="2020") + with temp_file( + content=header, name="test.py", change_into=True + ) as test_file: + code = update_file( + file=test_file, + copyright_regex=self.regex, + parsed_args=self.args, + term=term, + ) - test_file = self.path / "test.py" - if test_file.exists(): - test_file.unlink() - - test_file.write_text(header, encoding="utf-8") - - code = update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, - ) - - self.assertEqual(code, 0) - ret = mock_stdout.getvalue() - self.assertEqual( - ret, - f"{test_file}: Changed License Header " - "Copyright Year None -> 2021\n", - ) - self.assertIn( - "# SPDX-FileCopyrightText: 2020-2021 Greenbone AG", - test_file.read_text(encoding="utf-8"), - ) - - test_file.unlink() + self.assertEqual(code, 0) + ret = mock_stdout.getvalue() + self.assertEqual( + ret, + f"{test_file}: Changed License Header " + "Copyright Year None -> 2021\n", + ) + self.assertIn( + "# SPDX-FileCopyrightText: 2020-2021 Greenbone AG", + test_file.read_text(encoding="utf-8"), + ) @patch("sys.stdout", new_callable=StringIO) def test_update_header_ok_in_file(self, mock_stdout): @@ -412,32 +411,32 @@ def test_update_header_ok_in_file(self, mock_stdout): term = Terminal() header = HEADER.format(date="2021") + with temp_file( + content=header, name="test.py", change_into=True + ) as test_file: + code = update_file( + file=test_file, + copyright_regex=self.regex, + parsed_args=self.args, + term=term, + ) - test_file = self.path / "test.py" - if test_file.exists(): - test_file.unlink() - - test_file.write_text(header, encoding="utf-8") - - code = update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, - ) + self.assertEqual(code, 0) + ret = mock_stdout.getvalue() + self.assertEqual( + ret, + f"{test_file}: License Header is ok.\n", + ) + self.assertIn( + "# SPDX-FileCopyrightText: 2021 Greenbone AG", + test_file.read_text(encoding="utf-8"), + ) - self.assertEqual(code, 0) - ret = mock_stdout.getvalue() - self.assertEqual( - ret, - f"{test_file}: License Header is ok.\n", - ) - self.assertIn( - "# SPDX-FileCopyrightText: 2021 Greenbone AG", - test_file.read_text(encoding="utf-8"), - ) - test_file.unlink() +class ParseArgsTestCase(TestCase): + def setUp(self) -> None: + self.args = Namespace() + self.args.company = "Greenbone AG" def test_argparser_files(self): self.args.year = "2021" @@ -470,7 +469,7 @@ def test_argparser_dir(self): def test_get_exclude_list(self): # Try to find the current file from two directories up... - test_dirname = Path(__file__) / "../.." + test_dirname = Path(__file__).parent.parent.parent # with a relative glob test_ignore_file = Path("ignore.file") test_ignore_file.write_text("*.py\n", encoding="utf-8") @@ -483,6 +482,12 @@ def test_get_exclude_list(self): test_ignore_file.unlink() + +class MainTestCase(TestCase): + def setUp(self) -> None: + self.args = Namespace() + self.args.company = "Greenbone AG" + @patch("pontos.updateheader.updateheader.parse_args") def test_main(self, argparser_mock): self.args.year = "2021" From 96ff8516e3960ddfb291854dcdd6c2554c504eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 10:59:14 +0100 Subject: [PATCH 03/19] Change: Refactor update_header into a standalone function Strip code from update_header that is related to CLI arguments. --- pontos/updateheader/updateheader.py | 116 +++++----- tests/updateheader/test_header.py | 322 +++++++++++++--------------- 2 files changed, 213 insertions(+), 225 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index e673cd183..5fe3746d8 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -10,12 +10,11 @@ import re import sys -from argparse import Namespace +from dataclasses import dataclass from pathlib import Path from subprocess import CalledProcessError, run -from typing import Dict, List, Optional, Tuple, Union +from typing import List, Optional, Tuple, Union -from pontos.terminal import Terminal from pontos.terminal.null import NullTerminal from pontos.terminal.rich import RichTerminal @@ -79,20 +78,27 @@ def _get_modified_year(f: Path) -> str: raise e +@dataclass +class CopyrightMatch: + creation_year: str + modification_year: Optional[str] + company: str + + def _find_copyright( line: str, copyright_regex: re.Pattern, -) -> Tuple[bool, Union[Dict[str, Union[str, None]], None]]: +) -> Tuple[bool, Union[CopyrightMatch, None]]: """Match the line for the copyright_regex""" copyright_match = re.search(copyright_regex, line) if copyright_match: return ( True, - { - "creation_year": copyright_match.group(2), - "modification_year": copyright_match.group(3), - "company": copyright_match.group(4), - }, + CopyrightMatch( + creation_year=copyright_match.group(2), + modification_year=copyright_match.group(3), + company=copyright_match.group(4), + ), ) return False, None @@ -144,28 +150,19 @@ def _remove_outdated_lines( return None -def _update_file( +def update_file( file: Path, + year: str, + license_id: str, + company: str, copyright_regex: re.Pattern, - parsed_args: Namespace, - term: Terminal, cleanup_regexes: Optional[List[re.Pattern]] = None, -) -> int: - """Function to update the given file. - Checks if header exists. If not it adds an - header to that file, else it checks if year - is up to date - """ - - if parsed_args.changed: - try: - parsed_args.year = _get_modified_year(file) - except CalledProcessError: - term.warning( - f"{file}: Could not get date of last modification" - f" using git, using {str(parsed_args.year)} instead." - ) +) -> None: + """Function to update the header of the given file + Checks if header exists. If not it adds an header to that file, otherwise it + checks if year is up to date + """ try: with file.open("r+") as fp: found = False @@ -184,9 +181,9 @@ def _update_file( try: header = _add_header( file.suffix, - parsed_args.license_id, - parsed_args.company, - parsed_args.year, + license_id, + company, + year, ) if header: fp.seek(0) # back to beginning of file @@ -194,7 +191,8 @@ def _update_file( fp.seek(0) fp.write(header + "\n" + rest_of_file) print(f"{file}: Added license header.") - return 0 + return + except ValueError: print( f"{file}: No license header for the" @@ -202,21 +200,22 @@ def _update_file( ) except FileNotFoundError: print( - f"{file}: License file for {parsed_args.license_id} " + f"{file}: License file for {license_id} " "is not existing." ) - return 1 + return + # replace found header and write it to file if copyright_match and ( - not copyright_match["modification_year"] - and copyright_match["creation_year"] < parsed_args.year - or copyright_match["modification_year"] - and copyright_match["modification_year"] < parsed_args.year + not copyright_match.modification_year + and copyright_match.creation_year < year + or copyright_match.modification_year + and copyright_match.modification_year < year ): copyright_term = ( f"SPDX-FileCopyrightText: " - f'{copyright_match["creation_year"]}' - f"-{parsed_args.year} {parsed_args.company}" + f"{copyright_match.creation_year}" + f"-{year} {company}" ) new_line = re.sub(copyright_regex, copyright_term, line) fp_write = fp.tell() - len(line) # save position to insert @@ -230,8 +229,8 @@ def _update_file( fp.truncate() print( f"{file}: Changed License Header Copyright Year " - f'{copyright_match["modification_year"]} -> ' - f"{parsed_args.year}" + f"{copyright_match.modification_year} -> " + f"{year}" ) else: @@ -251,7 +250,6 @@ def _update_file( if new_content: file.write_text(new_content, encoding="utf-8") print(f"{file}: Cleaned up!") - return 0 def _get_exclude_list( @@ -312,8 +310,14 @@ def _compile_copyright_regex(company: Union[str, List[str]]) -> re.Pattern: def main() -> None: parsed_args = parse_args() exclude_list = [] - - if parsed_args.quiet: + year: str = parsed_args.year + license_id: str = parsed_args.license_id + company: str = parsed_args.company + changed: bool = parsed_args.changed + quiet: bool = parsed_args.quiet + cleanup: bool = parsed_args.cleanup + + if quiet: term: Union[NullTerminal, RichTerminal] = NullTerminal() else: term = RichTerminal() @@ -352,20 +356,30 @@ def main() -> None: ) cleanup_regexes: Optional[List[re.Pattern]] = None - if parsed_args.cleanup: + if cleanup: cleanup_regexes = _compile_outdated_regex() for file in files: + if changed: + try: + year = _get_modified_year(file) + except CalledProcessError: + term.warning( + f"{file}: Could not get date of last modification" + f" using git, using {year} instead." + ) + try: if file.absolute() in exclude_list: term.warning(f"{file}: Ignoring file from exclusion list.") else: - _update_file( - file=file, - copyright_regex=copyright_regex, - parsed_args=parsed_args, - term=term, - cleanup_regexes=cleanup_regexes, + update_file( + file, + year, + license_id, + company, + copyright_regex, + cleanup_regexes, ) except (FileNotFoundError, UnicodeDecodeError, ValueError): continue diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index b2ce1baab..a9a031ac5 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -21,6 +21,7 @@ _compile_copyright_regex, main, parse_args, + update_file, ) from pontos.updateheader.updateheader import _add_header as add_header from pontos.updateheader.updateheader import ( @@ -36,9 +37,6 @@ from pontos.updateheader.updateheader import ( _remove_outdated_lines as remove_outdated_lines, ) -from pontos.updateheader.updateheader import ( - _update_file as update_file, -) HEADER = """# SPDX-FileCopyrightText: {date} Greenbone AG # @@ -107,9 +105,9 @@ def test_find_copyright(self): ) self.assertTrue(found) self.assertIsNotNone(match) - self.assertEqual(match["creation_year"], "1995") - self.assertEqual(match["modification_year"], "2021") - self.assertEqual(match["company"], self.company) + self.assertEqual(match.creation_year, "1995") + self.assertEqual(match.modification_year, "2021") + self.assertEqual(match.company, self.company) # No modification Date found, match = find_copyright( @@ -117,9 +115,9 @@ def test_find_copyright(self): ) self.assertTrue(found) self.assertIsNotNone(match) - self.assertEqual(match["creation_year"], "1995") - self.assertEqual(match["modification_year"], None) - self.assertEqual(match["company"], self.company) + self.assertEqual(match.creation_year, "1995") + self.assertIsNone(match.modification_year) + self.assertEqual(match.company, self.company) # No match found, match = find_copyright( @@ -142,9 +140,9 @@ def test_find_spdx_copyright(self): ) self.assertTrue(found) self.assertIsNotNone(match) - self.assertEqual(match["creation_year"], "1995") - self.assertEqual(match["modification_year"], "2021") - self.assertEqual(match["company"], self.company) + self.assertEqual(match.creation_year, "1995") + self.assertEqual(match.modification_year, "2021") + self.assertEqual(match.company, self.company) # No modification Date found, match = find_copyright( @@ -152,9 +150,9 @@ def test_find_spdx_copyright(self): ) self.assertTrue(found) self.assertIsNotNone(match) - self.assertEqual(match["creation_year"], "1995") - self.assertEqual(match["modification_year"], None) - self.assertEqual(match["company"], self.company) + self.assertEqual(match.creation_year, "1995") + self.assertIsNone(match.modification_year) + self.assertEqual(match.company, self.company) # No match found, match = find_copyright( @@ -201,33 +199,30 @@ def test_add_header_license_not_found(self): class UpdateFileTestCase(TestCase): def setUp(self): - self.args = Namespace() - self.args.company = "Greenbone AG" + self.company = "Greenbone AG" self.path = Path(__file__).parent self.regex = re.compile( "(SPDX-FileCopyrightText:|[Cc]opyright).*?(19[0-9]{2}|20[0-9]{2}) " - f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.args.company})" + f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.company})" ) @patch("sys.stdout", new_callable=StringIO) def test_update_file_not_existing(self, mock_stdout): - self.args.year = "2020" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() + year = "2020" + license_id = "AGPL-3.0-or-later" with temp_directory(change_into=True) as temp_dir: test_file = temp_dir / "test.py" with self.assertRaises(FileNotFoundError): update_file( - file=test_file, + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) ret = mock_stdout.getvalue() @@ -238,20 +233,17 @@ def test_update_file_not_existing(self, mock_stdout): @patch("sys.stdout", new_callable=StringIO) def test_update_file_wrong_license(self, mock_stdout): - self.args.year = "2020" - self.args.changed = False - self.args.license_id = "AAAGPL-3.0-or-later" - - term = Terminal() + year = "2020" + license_id = "AAAGPL-3.0-or-later" with temp_file(name="test.py", change_into=True) as test_file: - code = update_file( - file=test_file, + update_file( + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) - self.assertEqual(code, 1) ret = mock_stdout.getvalue() self.assertEqual( @@ -262,20 +254,17 @@ def test_update_file_wrong_license(self, mock_stdout): @patch("sys.stdout", new_callable=StringIO) def test_update_file_suffix_invalid(self, mock_stdout): - self.args.year = "2020" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() + year = "2020" + license_id = "AGPL-3.0-or-later" with temp_file(name="test.pppy", change_into=True) as test_file: - code = update_file( - file=test_file, + update_file( + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) - self.assertEqual(code, 1) ret = mock_stdout.getvalue() self.assertEqual( @@ -285,11 +274,8 @@ def test_update_file_suffix_invalid(self, mock_stdout): @patch("sys.stdout", new_callable=StringIO) def test_update_file_binary_file(self, mock_stdout): - self.args.year = "2020" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() + year = "2020" + license_id = "AGPL-3.0-or-later" # create a Binary file ... # https://stackoverflow.com/a/30148554 @@ -299,13 +285,13 @@ def test_update_file_binary_file(self, mock_stdout): temp_file(name="test.py", content=data) as test_file, self.assertRaises(UnicodeDecodeError), ): - code = update_file( - file=test_file, + update_file( + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) - self.assertEqual(code, 1) ret = mock_stdout.getvalue() self.assertEqual( @@ -313,52 +299,20 @@ def test_update_file_binary_file(self, mock_stdout): f"{test_file}: Ignoring binary file.\n", ) - @patch("sys.stdout", new_callable=StringIO) - def test_update_file_changed(self, mock_stdout): - self.args.year = "1995" - self.args.changed = True - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() - - with ( - temp_directory(change_into=True) as temp_dir, - self.assertRaises(FileNotFoundError), - ): - test_file = temp_dir / "test.py" - - code = update_file( - file=test_file, - copyright_regex=self.regex, - parsed_args=self.args, - term=term, - ) - self.assertEqual(code, 1) - - ret = mock_stdout.getvalue() - - self.assertIn(f"{test_file}", ret) - self.assertIn("Could not get date", ret) - self.assertIn("of last modification using git,", ret) - self.assertIn(f"using {self.args.year} instead.", ret) - self.assertIn("File is not existing.", ret) - @patch("sys.stdout", new_callable=StringIO) def test_update_create_header(self, mock_stdout): - self.args.year = "1995" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() + year = "1995" + license_id = "AGPL-3.0-or-later" expected_header = HEADER.format(date="1995") + "\n\n" with temp_file(name="test.py", change_into=True) as test_file: - code = update_file( - file=test_file, + update_file( + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) ret = mock_stdout.getvalue() @@ -366,31 +320,27 @@ def test_update_create_header(self, mock_stdout): f"{test_file}: Added license header.\n", ret, ) - self.assertEqual(code, 0) self.assertEqual( expected_header, test_file.read_text(encoding="utf-8") ) @patch("sys.stdout", new_callable=StringIO) def test_update_header_in_file(self, mock_stdout): - self.args.year = "2021" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() + year = "2021" + license_id = "AGPL-3.0-or-later" header = HEADER.format(date="2020") with temp_file( content=header, name="test.py", change_into=True ) as test_file: - code = update_file( - file=test_file, + update_file( + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) - self.assertEqual(code, 0) ret = mock_stdout.getvalue() self.assertEqual( ret, @@ -404,24 +354,21 @@ def test_update_header_in_file(self, mock_stdout): @patch("sys.stdout", new_callable=StringIO) def test_update_header_ok_in_file(self, mock_stdout): - self.args.year = "2021" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - - term = Terminal() + year = "2021" + license_id = "AGPL-3.0-or-later" header = HEADER.format(date="2021") with temp_file( content=header, name="test.py", change_into=True ) as test_file: - code = update_file( - file=test_file, + update_file( + test_file, + year, + license_id, + self.company, copyright_regex=self.regex, - parsed_args=self.args, - term=term, ) - self.assertEqual(code, 0) ret = mock_stdout.getvalue() self.assertEqual( ret, @@ -432,6 +379,62 @@ def test_update_header_ok_in_file(self, mock_stdout): test_file.read_text(encoding="utf-8"), ) + def test_cleanup_file(self): + test_content = """# Copyright (C) 2021-2022 Greenbone Networks GmbH +# +# SPDX-License-Identifier: GPL-3.0-or-later +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import foo +import bar + +foo.baz(bar.boing) +""" # noqa: E501 + + expected_content = f"""# SPDX-FileCopyrightText: 2021-{str(datetime.datetime.now().year)} Greenbone AG +# +# SPDX-License-Identifier: GPL-3.0-or-later +# + +import foo +import bar + +foo.baz(bar.boing) +""" # noqa: E501 + + company = "Greenbone AG" + year = str(datetime.datetime.now().year) + license_id = "GPL-3.0-or-later" + compiled_regexes = compile_outdated_regex() + + with temp_file(content=test_content, name="foo.py") as tmp: + + update_file( + tmp, + year, + license_id, + company, + copyright_regex=_compile_copyright_regex( + ["Greenbone AG", OLD_COMPANY] + ), + cleanup_regexes=compiled_regexes, + ) + + new_content = tmp.read_text(encoding="utf-8") + self.assertEqual(expected_content, new_content) + class ParseArgsTestCase(TestCase): def setUp(self) -> None: @@ -533,8 +536,36 @@ def test_main_never_happen(self, argparser_mock, mock_stdout): ret, ) + @patch("sys.stdout", new_callable=StringIO) + @patch("pontos.updateheader.updateheader.parse_args") + def test_update_file_changed(self, argparser_mock, mock_stdout): + self.args.year = "1995" + self.args.license_id = "AGPL-3.0-or-later" + self.args.changed = True + self.args.directories = None + self.args.verbose = 0 + self.args.log_file = None + self.args.quiet = False + self.args.cleanup = False + + argparser_mock.return_value = self.args + + with temp_directory(change_into=True) as temp_dir: + test_file = temp_dir / "test.py" + self.args.files = str(test_file) + + main() + + ret = mock_stdout.getvalue() -class UpdateHeaderCleanupTestCase(TestCase): + self.assertIn(f"{test_file}", ret) + self.assertIn("Could not get date", ret) + self.assertIn("of last modification using git,", ret) + self.assertIn(f"using {self.args.year} instead.", ret) + self.assertIn("File is not existing.", ret) + + +class RemoveOutdatedLinesTestCase(TestCase): def setUp(self) -> None: self.compiled_regexes = compile_outdated_regex() @@ -571,60 +602,3 @@ def test_remove_outdated_lines2(self): content=test_content, cleanup_regexes=self.compiled_regexes ) self.assertEqual(new_content, "\n") - - def test_cleanup_file(self): - test_content = """# Copyright (C) 2021-2022 Greenbone Networks GmbH -# -# SPDX-License-Identifier: GPL-3.0-or-later -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -import foo -import bar - -foo.baz(bar.boing) -""" # noqa: E501 - - expected_content = f"""# SPDX-FileCopyrightText: 2021-{str(datetime.datetime.now().year)} Greenbone AG -# -# SPDX-License-Identifier: GPL-3.0-or-later -# - -import foo -import bar - -foo.baz(bar.boing) -""" # noqa: E501 - - with temp_file(content=test_content, name="foo.py") as tmp: - args = Namespace() - args.company = "Greenbone AG" - args.year = str(datetime.datetime.now().year) - args.changed = False - args.license_id = "GPL-3.0-or-later" - args.verbose = 0 - args.cleanup = True - - update_file( - file=tmp, - copyright_regex=_compile_copyright_regex( - ["Greenbone AG", OLD_COMPANY] - ), - parsed_args=args, - term=Terminal(), - cleanup_regexes=self.compiled_regexes, - ) - - new_content = tmp.read_text(encoding="utf-8") - self.assertEqual(expected_content, new_content) From c1a719e7a5d357fa9c2cb256b6c1bdbdd1ef5bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 11:26:09 +0100 Subject: [PATCH 04/19] Use Git class in _get_modified_year We have a dedicated class for handling git related tasks. Therefore we should use it here too. --- pontos/updateheader/updateheader.py | 18 ++++-------------- tests/updateheader/test_header.py | 26 ++++++++------------------ 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index 5fe3746d8..16575781d 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -12,9 +12,10 @@ import sys from dataclasses import dataclass from pathlib import Path -from subprocess import CalledProcessError, run from typing import List, Optional, Tuple, Union +from pontos.errors import PontosError +from pontos.git import Git from pontos.terminal.null import NullTerminal from pontos.terminal.rich import RichTerminal @@ -64,18 +65,7 @@ def _get_modified_year(f: Path) -> str: """In case of the changed arg, update year to last modified year""" - try: - cmd = ["git", "log", "-1", "--format=%ad", "--date=format:%Y", str(f)] - proc = run( - cmd, - text=True, - capture_output=True, - check=True, - universal_newlines=True, - ) - return proc.stdout.rstrip() - except CalledProcessError as e: - raise e + return Git().log("-1", "--date=format:%Y", str(f), format="%ad")[0] @dataclass @@ -363,7 +353,7 @@ def main() -> None: if changed: try: year = _get_modified_year(file) - except CalledProcessError: + except PontosError: term.warning( f"{file}: Could not get date of last modification" f" using git, using {year} instead." diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index a9a031ac5..91f1308de 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -10,10 +10,10 @@ from contextlib import redirect_stdout from io import StringIO from pathlib import Path -from subprocess import CalledProcessError, CompletedProcess from unittest import TestCase -from unittest.mock import patch +from unittest.mock import MagicMock, patch +from pontos.errors import PontosError from pontos.terminal.terminal import ConsoleTerminal from pontos.testing import temp_directory, temp_file from pontos.updateheader.updateheader import ( @@ -53,22 +53,12 @@ def get_width() -> int: class GetModifiedYearTestCase(TestCase): - @patch("pontos.updateheader.updateheader.run") - def test_get_modified_year(self, run_mock): + @patch("pontos.updateheader.updateheader.Git") + def test_get_modified_year(self, git_mock): with temp_file(name="test.py", change_into=True) as test_file: - run_mock.return_value = CompletedProcess( - args=[ - "git", - "log", - "-1", - "--format=%ad", - "--date=format:%Y", - f"{test_file}", - ], - returncode=0, - stdout="2020\n", - stderr="", - ) + git_instance_mock = MagicMock() + git_instance_mock.log.return_value = ["2020"] + git_mock.return_value = git_instance_mock year = get_modified_year(f=test_file) self.assertEqual(year, "2020") @@ -76,7 +66,7 @@ def test_get_modified_year(self, run_mock): def test_get_modified_year_error(self): with ( temp_directory(change_into=True) as temp_dir, - self.assertRaises(CalledProcessError), + self.assertRaises(PontosError), ): test_file = temp_dir / "test.py" From da9ca91e093c30800b10faceaf6fb00c95d004cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 11:43:02 +0100 Subject: [PATCH 05/19] Add: Add a testcase for parsing defaults for update header CLI Ensure that the defaults are set and parsed correctly. --- tests/updateheader/test_header.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index 91f1308de..e42c35d32 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -475,6 +475,21 @@ def test_get_exclude_list(self): test_ignore_file.unlink() + def test_defaults(self): + args = ["-f", "foo.txt"] + args = parse_args(args) + + self.assertFalse(args.quiet) + self.assertIsNone(args.log_file) + self.assertFalse(args.changed) + self.assertEqual(args.year, str(datetime.date.today().year)) + self.assertEqual(args.license_id, "GPL-3.0-or-later") + self.assertEqual(args.company, "Greenbone AG") + self.assertEqual(args.files, ["foo.txt"]) + self.assertIsNone(args.directories) + self.assertIsNone(args.exclude_file) + self.assertFalse(args.cleanup) + class MainTestCase(TestCase): def setUp(self) -> None: From 747a728baadf5461cfff9287650d1cff115ed40f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:02:28 +0100 Subject: [PATCH 06/19] Further tests cleanup Extract testing of get_exclude_list into an own test case and simplify parse_args test case. --- tests/updateheader/test_header.py | 60 +++++++++++++------------------ 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index e42c35d32..f1366fbe1 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -427,53 +427,24 @@ def test_cleanup_file(self): class ParseArgsTestCase(TestCase): - def setUp(self) -> None: - self.args = Namespace() - self.args.company = "Greenbone AG" - def test_argparser_files(self): - self.args.year = "2021" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - args = ["-f", "test.py", "-y", "2021", "-l", "AGPL-3.0-or-later"] - args = parse_args(args) - self.assertIsNotNone(args) - self.assertEqual(args.company, self.args.company) + + self.assertEqual(args.company, "Greenbone AG") self.assertEqual(args.files, ["test.py"]) - self.assertEqual(args.year, self.args.year) - self.assertEqual(args.license_id, self.args.license_id) + self.assertEqual(args.year, "2021") + self.assertEqual(args.license_id, "AGPL-3.0-or-later") def test_argparser_dir(self): - self.args.year = "2020" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - args = ["-d", ".", "-c", "-l", "AGPL-3.0-or-later"] - args = parse_args(args) - self.assertIsNotNone(args) - self.assertEqual(args.company, self.args.company) + self.assertEqual(args.directories, ["."]) + self.assertEqual(args.company, "Greenbone AG") self.assertTrue(args.changed) self.assertEqual(args.year, str(datetime.datetime.now().year)) - self.assertEqual(args.license_id, self.args.license_id) - - def test_get_exclude_list(self): - # Try to find the current file from two directories up... - test_dirname = Path(__file__).parent.parent.parent - # with a relative glob - test_ignore_file = Path("ignore.file") - test_ignore_file.write_text("*.py\n", encoding="utf-8") - - exclude_list = get_exclude_list( - test_ignore_file, [test_dirname.resolve()] - ) - - self.assertIn(Path(__file__), exclude_list) - - test_ignore_file.unlink() + self.assertEqual(args.license_id, "AGPL-3.0-or-later") def test_defaults(self): args = ["-f", "foo.txt"] @@ -491,6 +462,23 @@ def test_defaults(self): self.assertFalse(args.cleanup) +class GetExcludeListTestCase(TestCase): + def test_get_exclude_list(self): + # Try to find the current file from two directories up... + test_dirname = Path(__file__).parent.parent.parent + # with a relative glob + test_ignore_file = Path("ignore.file") + test_ignore_file.write_text("*.py\n", encoding="utf-8") + + exclude_list = get_exclude_list( + test_ignore_file, [test_dirname.resolve()] + ) + + self.assertIn(Path(__file__), exclude_list) + + test_ignore_file.unlink() + + class MainTestCase(TestCase): def setUp(self) -> None: self.args = Namespace() From 10d090449c68fb8c4ae20a83f6856fc672e0daae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:09:40 +0100 Subject: [PATCH 07/19] Change: Remove copyright_regex from update_header function The update_header function just expect a year, license and company name at the end. --- pontos/updateheader/updateheader.py | 9 +++------ tests/updateheader/test_header.py | 26 +++----------------------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index 16575781d..dd1bb89b9 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -145,7 +145,6 @@ def update_file( year: str, license_id: str, company: str, - copyright_regex: re.Pattern, cleanup_regexes: Optional[List[re.Pattern]] = None, ) -> None: """Function to update the header of the given file @@ -153,6 +152,9 @@ def update_file( Checks if header exists. If not it adds an header to that file, otherwise it checks if year is up to date """ + + copyright_regex = _compile_copyright_regex(company=[company, OLD_COMPANY]) + try: with file.open("r+") as fp: found = False @@ -341,10 +343,6 @@ def main() -> None: term.error("Specify files to update!") sys.exit(1) - copyright_regex: re.Pattern = _compile_copyright_regex( - company=[parsed_args.company, OLD_COMPANY] - ) - cleanup_regexes: Optional[List[re.Pattern]] = None if cleanup: cleanup_regexes = _compile_outdated_regex() @@ -368,7 +366,6 @@ def main() -> None: year, license_id, company, - copyright_regex, cleanup_regexes, ) except (FileNotFoundError, UnicodeDecodeError, ValueError): diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index f1366fbe1..f9eabfd61 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -4,7 +4,6 @@ # import datetime -import re import struct from argparse import Namespace from contextlib import redirect_stdout @@ -16,14 +15,13 @@ from pontos.errors import PontosError from pontos.terminal.terminal import ConsoleTerminal from pontos.testing import temp_directory, temp_file +from pontos.updateheader.updateheader import _add_header as add_header from pontos.updateheader.updateheader import ( - OLD_COMPANY, _compile_copyright_regex, main, parse_args, update_file, ) -from pontos.updateheader.updateheader import _add_header as add_header from pontos.updateheader.updateheader import ( _compile_outdated_regex as compile_outdated_regex, ) @@ -74,12 +72,9 @@ def test_get_modified_year_error(self): class FindCopyRightTestCase(TestCase): - def setUp(self): + def setUp(self) -> None: self.company = "Greenbone AG" - self.regex = re.compile( - "(SPDX-FileCopyrightText:|[Cc]opyright).*?(19[0-9]{2}|20[0-9]{2}) " - f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.company})" - ) + self.regex = _compile_copyright_regex(self.company) def test_find_copyright(self): test_line = "# Copyright (C) 1995-2021 Greenbone AG" @@ -193,11 +188,6 @@ def setUp(self): self.path = Path(__file__).parent - self.regex = re.compile( - "(SPDX-FileCopyrightText:|[Cc]opyright).*?(19[0-9]{2}|20[0-9]{2}) " - f"?-? ?(19[0-9]{{2}}|20[0-9]{{2}})? ({self.company})" - ) - @patch("sys.stdout", new_callable=StringIO) def test_update_file_not_existing(self, mock_stdout): year = "2020" @@ -212,7 +202,6 @@ def test_update_file_not_existing(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -232,7 +221,6 @@ def test_update_file_wrong_license(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -253,7 +241,6 @@ def test_update_file_suffix_invalid(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -280,7 +267,6 @@ def test_update_file_binary_file(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -302,7 +288,6 @@ def test_update_create_header(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -328,7 +313,6 @@ def test_update_header_in_file(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -356,7 +340,6 @@ def test_update_header_ok_in_file(self, mock_stdout): year, license_id, self.company, - copyright_regex=self.regex, ) ret = mock_stdout.getvalue() @@ -416,9 +399,6 @@ def test_cleanup_file(self): year, license_id, company, - copyright_regex=_compile_copyright_regex( - ["Greenbone AG", OLD_COMPANY] - ), cleanup_regexes=compiled_regexes, ) From e7333577e2bf408ae7543e8870c70d7d13a0051c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:26:16 +0100 Subject: [PATCH 08/19] Simplify _get_exclude_list It's more likely that the exclude file doesn't exist. Therefore it is better to check is the file exists then reacting on the file not found exception. --- pontos/updateheader/updateheader.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index dd1bb89b9..1006d7ca6 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -245,8 +245,8 @@ def update_file( def _get_exclude_list( - exclude_file: Path, directories: List[Path] -) -> List[Path]: + exclude_file: Path, directories: list[Path] +) -> list[Path]: """Tries to get the list of excluded files / directories. If a file is given, it will be used. Otherwise it will be searched in the executed root path. @@ -256,12 +256,12 @@ def _get_exclude_list( if exclude_file is None: exclude_file = Path(".pontos-header-ignore") - try: - exclude_lines = exclude_file.read_text(encoding="utf-8").split("\n") - except FileNotFoundError: - print("No exclude list file found.") + + if not exclude_file.is_file(): return [] + exclude_lines = exclude_file.read_text(encoding="utf-8").splitlines() + expanded_globs = [ directory.rglob(line.strip()) for directory in directories From 0fce4f800cedf228663d13fe00b9a95bfef70094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:29:31 +0100 Subject: [PATCH 09/19] Use list comprehension for _compile_outdated_regex For easy iterations that create a list from a list it's better to use list comprehension and also it should be faster. --- pontos/updateheader/updateheader.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index 1006d7ca6..c2eb839f5 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -281,12 +281,9 @@ def _get_exclude_list( return exclude_list -def _compile_outdated_regex() -> List[re.Pattern]: +def _compile_outdated_regex() -> list[re.Pattern]: """prepare regex patterns to remove old copyright lines""" - regexes: List[re.Pattern] = [] - for line in OLD_LINES: - regexes.append(re.compile(rf"^(([#*]|//) ?)?{line}")) - return regexes + return [re.compile(rf"^(([#*]|//) ?)?{line}") for line in OLD_LINES] def _compile_copyright_regex(company: Union[str, List[str]]) -> re.Pattern: From d38e2a9a6c4474e7b5c5b4fe008aab7eed524e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:32:18 +0100 Subject: [PATCH 10/19] Use list and tuple instead of List and Tuple List and Tuple are obsolete with PEP 585 and Python 3.9. --- pontos/updateheader/updateheader.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index c2eb839f5..f1c14306c 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -12,7 +12,7 @@ import sys from dataclasses import dataclass from pathlib import Path -from typing import List, Optional, Tuple, Union +from typing import Optional, Union from pontos.errors import PontosError from pontos.git import Git @@ -78,7 +78,7 @@ class CopyrightMatch: def _find_copyright( line: str, copyright_regex: re.Pattern, -) -> Tuple[bool, Union[CopyrightMatch, None]]: +) -> tuple[bool, Union[CopyrightMatch, None]]: """Match the line for the copyright_regex""" copyright_match = re.search(copyright_regex, line) if copyright_match: @@ -117,7 +117,7 @@ def _add_header( def _remove_outdated_lines( - content: str, cleanup_regexes: List[re.Pattern] + content: str, cleanup_regexes: list[re.Pattern] ) -> Optional[str]: """Remove lines that contain outdated copyright header ...""" changed = False @@ -145,7 +145,7 @@ def update_file( year: str, license_id: str, company: str, - cleanup_regexes: Optional[List[re.Pattern]] = None, + cleanup_regexes: Optional[list[re.Pattern]] = None, ) -> None: """Function to update the header of the given file @@ -286,7 +286,7 @@ def _compile_outdated_regex() -> list[re.Pattern]: return [re.compile(rf"^(([#*]|//) ?)?{line}") for line in OLD_LINES] -def _compile_copyright_regex(company: Union[str, List[str]]) -> re.Pattern: +def _compile_copyright_regex(company: Union[str, list[str]]) -> re.Pattern: """prepare the copyright regex""" c_str = r"(SPDX-FileCopyrightText:|[Cc]opyright)" d_str = r"(19[0-9]{2}|20[0-9]{2})" @@ -340,7 +340,7 @@ def main() -> None: term.error("Specify files to update!") sys.exit(1) - cleanup_regexes: Optional[List[re.Pattern]] = None + cleanup_regexes: Optional[list[re.Pattern]] = None if cleanup: cleanup_regexes = _compile_outdated_regex() From 06a9ba71f3db60b7cff96da19d227a0c13a33efa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:33:48 +0100 Subject: [PATCH 11/19] Remove obsolete Terminal mock class in update header test module --- tests/updateheader/test_header.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index f9eabfd61..df9952bc8 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -13,7 +13,6 @@ from unittest.mock import MagicMock, patch from pontos.errors import PontosError -from pontos.terminal.terminal import ConsoleTerminal from pontos.testing import temp_directory, temp_file from pontos.updateheader.updateheader import _add_header as add_header from pontos.updateheader.updateheader import ( @@ -41,15 +40,6 @@ # SPDX-License-Identifier: AGPL-3.0-or-later""" -_here_ = Path(__file__).parent - - -class Terminal(ConsoleTerminal): - @staticmethod - def get_width() -> int: - return 999 - - class GetModifiedYearTestCase(TestCase): @patch("pontos.updateheader.updateheader.Git") def test_get_modified_year(self, git_mock): From 73d60cb08ddf710ef00998484be6091a9a68dd2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 12:34:36 +0100 Subject: [PATCH 12/19] Change: Improve and finalize update_file function API Don't pass the cleanup_regexes to the function and instead just add a flag to cleanup the updated file instead. --- pontos/updateheader/updateheader.py | 10 ++++------ tests/updateheader/test_header.py | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index f1c14306c..364fd620e 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -145,7 +145,8 @@ def update_file( year: str, license_id: str, company: str, - cleanup_regexes: Optional[list[re.Pattern]] = None, + *, + cleanup: bool = False, ) -> None: """Function to update the header of the given file @@ -154,6 +155,7 @@ def update_file( """ copyright_regex = _compile_copyright_regex(company=[company, OLD_COMPANY]) + cleanup_regexes = _compile_outdated_regex() if cleanup else None try: with file.open("r+") as fp: @@ -340,10 +342,6 @@ def main() -> None: term.error("Specify files to update!") sys.exit(1) - cleanup_regexes: Optional[list[re.Pattern]] = None - if cleanup: - cleanup_regexes = _compile_outdated_regex() - for file in files: if changed: try: @@ -363,7 +361,7 @@ def main() -> None: year, license_id, company, - cleanup_regexes, + cleanup=cleanup, ) except (FileNotFoundError, UnicodeDecodeError, ValueError): continue diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index df9952bc8..aaa9034a5 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -380,7 +380,6 @@ def test_cleanup_file(self): company = "Greenbone AG" year = str(datetime.datetime.now().year) license_id = "GPL-3.0-or-later" - compiled_regexes = compile_outdated_regex() with temp_file(content=test_content, name="foo.py") as tmp: @@ -389,7 +388,7 @@ def test_cleanup_file(self): year, license_id, company, - cleanup_regexes=compiled_regexes, + cleanup=True, ) new_content = tmp.read_text(encoding="utf-8") From 22394825df4231f5d2beeace96409a1e0975269f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 13:26:08 +0100 Subject: [PATCH 13/19] Add: Add test for update-header passing files and directories --- tests/updateheader/test_header.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index aaa9034a5..aec923471 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -430,6 +430,16 @@ def test_defaults(self): self.assertIsNone(args.exclude_file) self.assertFalse(args.cleanup) + def test_files_and_directories_mutual_exclusive(self): + args = ["--files", "foo", "--directories", "bar"] + with self.assertRaises(SystemExit) as cm: + args = parse_args(args) + + self.assertIn( + "argument -d/--directories: not allowed with argument -f/--file", + cm.msg, + ) + class GetExcludeListTestCase(TestCase): def test_get_exclude_list(self): From 02875d6caf562c8b3b44e15f901630654f839bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 13:27:52 +0100 Subject: [PATCH 14/19] Change: Improve help for update-header arguments Improved usage and print also the defaults. --- pontos/updateheader/_parser.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pontos/updateheader/_parser.py b/pontos/updateheader/_parser.py index 7f059b8c5..f3e349029 100644 --- a/pontos/updateheader/_parser.py +++ b/pontos/updateheader/_parser.py @@ -47,7 +47,8 @@ def parse_args(args: Optional[Sequence[str]] = None) -> Namespace: default=False, help=( "Update modified year using git log modified year. " - "This will not changed all files to current year!" + "Used instead of --year. If the modified year could not be " + "determined via git it falls back to --year." ), ) date_group.add_argument( @@ -56,7 +57,7 @@ def parse_args(args: Optional[Sequence[str]] = None) -> Namespace: default=str(datetime.now().year), help=( "If year is set, modified year will be " - "set to the specified year." + "set to the specified year. Default is %(default)s." ), ) @@ -66,7 +67,7 @@ def parse_args(args: Optional[Sequence[str]] = None) -> Namespace: dest="license_id", choices=SUPPORTED_LICENSES, default="GPL-3.0-or-later", - help=("Use the passed license type"), + help="Use the passed license type. Default is %(default)s", ) parser.add_argument( @@ -74,7 +75,7 @@ def parse_args(args: Optional[Sequence[str]] = None) -> Namespace: default="Greenbone AG", help=( "If a header will be added to file, " - "it will be licensed by company." + "it will be licensed by company. Default is %(default)s" ), ) From f00c19d83f9320f23db598d78f046201dfb7eae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 13:29:46 +0100 Subject: [PATCH 15/19] Change: Allow to pass --year and --changed for update-header CLI --year is the fallback of --changed if the year of the last git modification can't be calculated. Therefore it should be allowed to be set in conjunction with --changed. --- pontos/updateheader/_parser.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pontos/updateheader/_parser.py b/pontos/updateheader/_parser.py index f3e349029..a14bc5869 100644 --- a/pontos/updateheader/_parser.py +++ b/pontos/updateheader/_parser.py @@ -39,8 +39,7 @@ def parse_args(args: Optional[Sequence[str]] = None) -> Namespace: help="Activate logging using the given file path", ).complete = shtab.FILE # type: ignore[attr-defined] - date_group = parser.add_mutually_exclusive_group() - date_group.add_argument( + parser.add_argument( "-c", "--changed", action="store_true", @@ -51,7 +50,7 @@ def parse_args(args: Optional[Sequence[str]] = None) -> Namespace: "determined via git it falls back to --year." ), ) - date_group.add_argument( + parser.add_argument( "-y", "--year", default=str(datetime.now().year), From e9101b5f0ed34352634401f3aac717abd38f8233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 13:33:11 +0100 Subject: [PATCH 16/19] Easier testing for update-header main function --- pontos/updateheader/updateheader.py | 8 ++-- tests/updateheader/test_header.py | 69 ++++++++++++----------------- 2 files changed, 33 insertions(+), 44 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index 364fd620e..452603eab 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -12,7 +12,7 @@ import sys from dataclasses import dataclass from pathlib import Path -from typing import Optional, Union +from typing import Optional, Sequence, Union from pontos.errors import PontosError from pontos.git import Git @@ -298,8 +298,8 @@ def _compile_copyright_regex(company: Union[str, list[str]]) -> re.Pattern: return re.compile(rf"{c_str}.*? {d_str}?-? ?{d_str}? ({'|'.join(company)})") -def main() -> None: - parsed_args = parse_args() +def main(args: Optional[Sequence[str]] = None) -> None: + parsed_args = parse_args(args) exclude_list = [] year: str = parsed_args.year license_id: str = parsed_args.license_id @@ -349,7 +349,7 @@ def main() -> None: except PontosError: term.warning( f"{file}: Could not get date of last modification" - f" using git, using {year} instead." + f" via git, using {year} instead." ) try: diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index aec923471..44f0753a4 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -463,25 +463,17 @@ def setUp(self) -> None: self.args = Namespace() self.args.company = "Greenbone AG" - @patch("pontos.updateheader.updateheader.parse_args") - def test_main(self, argparser_mock): - self.args.year = "2021" - self.args.changed = False - self.args.license_id = "AGPL-3.0-or-later" - self.args.files = ["test.py"] - self.args.directories = None - self.args.verbose = 0 - self.args.log_file = None - self.args.quiet = False - self.args.cleanup = False - - argparser_mock.return_value = self.args - + def test_main(self): + args = [ + "--year", + "2021", + "--license", + "AGPL-3.0-or-later", + "--files", + "test.py", + ] with redirect_stdout(StringIO()): - code = True if not main() else False - - # I have no idea how or why test main ... - self.assertTrue(code) + main(args) @patch("sys.stdout", new_callable=StringIO) @patch("pontos.updateheader.updateheader.parse_args") @@ -508,33 +500,30 @@ def test_main_never_happen(self, argparser_mock, mock_stdout): ret, ) - @patch("sys.stdout", new_callable=StringIO) - @patch("pontos.updateheader.updateheader.parse_args") - def test_update_file_changed(self, argparser_mock, mock_stdout): - self.args.year = "1995" - self.args.license_id = "AGPL-3.0-or-later" - self.args.changed = True - self.args.directories = None - self.args.verbose = 0 - self.args.log_file = None - self.args.quiet = False - self.args.cleanup = False - - argparser_mock.return_value = self.args + def test_update_file_changed_no_git(self): + args = [ + "--changed", + "--year", + "1999", + "--files", + ] - with temp_directory(change_into=True) as temp_dir: + with ( + redirect_stdout(StringIO()) as out, + temp_directory(change_into=True) as temp_dir, + ): test_file = temp_dir / "test.py" - self.args.files = str(test_file) + args.append(str(test_file)) - main() + main(args) - ret = mock_stdout.getvalue() + ret = out.getvalue() - self.assertIn(f"{test_file}", ret) - self.assertIn("Could not get date", ret) - self.assertIn("of last modification using git,", ret) - self.assertIn(f"using {self.args.year} instead.", ret) - self.assertIn("File is not existing.", ret) + self.assertIn( + "Could not get date of last modification via git, " + f"using 1999 instead.{test_file}: File is not existing.", + ret.replace("\n", ""), + ) class RemoveOutdatedLinesTestCase(TestCase): From 614c2dad9786494ea36c389ecce7260b65f7d6a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 13:48:21 +0100 Subject: [PATCH 17/19] Add a test for updating a header with spdx copyright --- tests/updateheader/test_header.py | 42 +++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index 44f0753a4..9bc3b41f8 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -173,6 +173,8 @@ def test_add_header_license_not_found(self): class UpdateFileTestCase(TestCase): + maxDiff = None + def setUp(self): self.company = "Greenbone AG" @@ -374,6 +376,46 @@ def test_cleanup_file(self): import foo import bar +foo.baz(bar.boing) +""" # noqa: E501 + + company = "Greenbone AG" + year = str(datetime.datetime.now().year) + license_id = "GPL-3.0-or-later" + + with temp_file(content=test_content, name="foo.py") as tmp: + + update_file( + tmp, + year, + license_id, + company, + cleanup=True, + ) + + new_content = tmp.read_text(encoding="utf-8") + self.assertEqual(expected_content, new_content) + + def test_cleanup_file_spdx_header(self): + test_content = """ +# SPDX-FileCopyrightText: 2021 Greenbone AG +# +# SPDX-License-Identifier: GPL-3.0-or-later + +import foo +import bar + +foo.baz(bar.boing) +""" # noqa: E501 + + expected_content = f""" +# SPDX-FileCopyrightText: 2021-{str(datetime.datetime.now().year)} Greenbone AG +# +# SPDX-License-Identifier: GPL-3.0-or-later + +import foo +import bar + foo.baz(bar.boing) """ # noqa: E501 From 4bcbe9f0490f7e6d0c9760a18f67dc64f5c93627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Fri, 23 Feb 2024 13:57:42 +0100 Subject: [PATCH 18/19] Fix: Fix overriding company in copyright header via update-header CLI We need a regex that matches all kind of company names not only Greenbone. This also generalizes the code and makes update-header independent of Greenbone specific use cases. --- pontos/updateheader/updateheader.py | 9 +++---- tests/updateheader/test_header.py | 42 ++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index 452603eab..fcac6a360 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -60,7 +60,6 @@ "along with this program; if not, write to the Free Software", "Foundation, Inc\., 51 Franklin St, Fifth Floor, Boston, MA 02110\-1301 USA\.", # noqa: E501 ] -OLD_COMPANY = "Greenbone Networks GmbH" def _get_modified_year(f: Path) -> str: @@ -154,7 +153,7 @@ def update_file( checks if year is up to date """ - copyright_regex = _compile_copyright_regex(company=[company, OLD_COMPANY]) + copyright_regex = _compile_copyright_regex() cleanup_regexes = _compile_outdated_regex() if cleanup else None try: @@ -288,14 +287,12 @@ def _compile_outdated_regex() -> list[re.Pattern]: return [re.compile(rf"^(([#*]|//) ?)?{line}") for line in OLD_LINES] -def _compile_copyright_regex(company: Union[str, list[str]]) -> re.Pattern: +def _compile_copyright_regex() -> re.Pattern: """prepare the copyright regex""" c_str = r"(SPDX-FileCopyrightText:|[Cc]opyright)" d_str = r"(19[0-9]{2}|20[0-9]{2})" - if isinstance(company, str): - return re.compile(rf"{c_str}.*? {d_str}?-? ?{d_str}? ({company})") - return re.compile(rf"{c_str}.*? {d_str}?-? ?{d_str}? ({'|'.join(company)})") + return re.compile(rf"{c_str}.*? {d_str}?-? ?{d_str}? (.+)") def main(args: Optional[Sequence[str]] = None) -> None: diff --git a/tests/updateheader/test_header.py b/tests/updateheader/test_header.py index 9bc3b41f8..560fda065 100644 --- a/tests/updateheader/test_header.py +++ b/tests/updateheader/test_header.py @@ -64,7 +64,7 @@ def test_get_modified_year_error(self): class FindCopyRightTestCase(TestCase): def setUp(self) -> None: self.company = "Greenbone AG" - self.regex = _compile_copyright_regex(self.company) + self.regex = _compile_copyright_regex() def test_find_copyright(self): test_line = "# Copyright (C) 1995-2021 Greenbone AG" @@ -436,6 +436,46 @@ def test_cleanup_file_spdx_header(self): new_content = tmp.read_text(encoding="utf-8") self.assertEqual(expected_content, new_content) + def test_cleanup_file_changed_company(self): + test_content = """ +# SPDX-FileCopyrightText: 2021 Greenbone AG +# +# SPDX-License-Identifier: GPL-3.0-or-later + +import foo +import bar + +foo.baz(bar.boing) +""" # noqa: E501 + + expected_content = f""" +# SPDX-FileCopyrightText: 2021-{str(datetime.datetime.now().year)} ACME Inc. +# +# SPDX-License-Identifier: GPL-3.0-or-later + +import foo +import bar + +foo.baz(bar.boing) +""" # noqa: E501 + + company = "ACME Inc." + year = str(datetime.datetime.now().year) + license_id = "GPL-3.0-or-later" + + with temp_file(content=test_content, name="foo.py") as tmp: + + update_file( + tmp, + year, + license_id, + company, + cleanup=True, + ) + + new_content = tmp.read_text(encoding="utf-8") + self.assertEqual(expected_content, new_content) + class ParseArgsTestCase(TestCase): def test_argparser_files(self): From 2a394c333493b41a9ef5e78524bebf2ef0609842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Ricks?= Date: Mon, 26 Feb 2024 08:42:19 +0100 Subject: [PATCH 19/19] Only compile regex pattern once Improve the performance for creating the regexp pattern by caching them. With this change they will be created on demand once and afterwards they are used from the cache. --- pontos/updateheader/updateheader.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pontos/updateheader/updateheader.py b/pontos/updateheader/updateheader.py index fcac6a360..eb4967644 100644 --- a/pontos/updateheader/updateheader.py +++ b/pontos/updateheader/updateheader.py @@ -11,6 +11,7 @@ import re import sys from dataclasses import dataclass +from functools import cache from pathlib import Path from typing import Optional, Sequence, Union @@ -282,11 +283,13 @@ def _get_exclude_list( return exclude_list +@cache def _compile_outdated_regex() -> list[re.Pattern]: """prepare regex patterns to remove old copyright lines""" return [re.compile(rf"^(([#*]|//) ?)?{line}") for line in OLD_LINES] +@cache def _compile_copyright_regex() -> re.Pattern: """prepare the copyright regex""" c_str = r"(SPDX-FileCopyrightText:|[Cc]opyright)"