Merge pull request #1710 from MVrachev/start-linting

Start linting the test

docs/CONTRIBUTORS.rst

@@ -139,9 +139,9 @@ Individual tests can also be executed. Optional '-v' flags can be added to
 increase log level up to DEBUG ('-vvvv').
 ::
 
-    $ python3 test_updater.py # run a specific test file
-    $ python3 test_updater.py TestUpdater.test_4_refresh # run a specific test
-    $ python3 test_updater.py -vvvv TestUpdater.test_4_refresh # run test with DEBUG log level
+    $ python3 test_updater_ng.py # run a specific test file
+    $ python3 test_updater_ng.py TestUpdater.test_refresh_and_download # run a specific test
+    $ python3 test_updater_ng.py -vvvv TestUpdater.test_refresh_and_download # run test with DEBUG log level
 
 
 All of the log levels and the corresponding options that could be used for testing are:

pyproject.toml

@@ -7,13 +7,17 @@ build-backend = "setuptools.build_meta"
 # Read more here: https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-via-a-file
 [tool.black]
 line-length=80
+# TODO: remove "excludes" after deleting old test files
+exclude="tests/.*old.py"
 
 # Isort section
 # Read more here: https://pycqa.github.io/isort/docs/configuration/config_files.html
 [tool.isort]
 profile="black"
 line_length=80
 known_first_party = ["tuf"]
+# TODO: remove "skip_glob" after deleting old test files
+skip_glob="*old.py"
 
 # Pylint section
 
@@ -55,6 +59,8 @@ module-rgx="^(_?[a-z][a-z0-9_]*|__init__)$"
 no-docstring-rgx="(__.*__|main|test.*|.*test|.*Test)$"
 variable-rgx="^[a-z][a-z0-9_]*$"
 docstring-min-length=10
+# TODO: remove "ignore-patterns" after deleting old test files
+ignore-patterns=".*_old.py"
 
 [tool.pylint.logging]
 logging-format-style="old"
@@ -76,6 +82,9 @@ strict_equality = "True"
 disallow_untyped_defs = "True"
 disallow_untyped_calls = "True"
 show_error_codes = "True"
+disable_error_code = ["attr-defined"]
+# TODO: remove "exclude" after deleting old test files
+exclude=".*_old.py"
 
 [[tool.mypy.overrides]]
 module = [

requirements-test.txt

@@ -14,3 +14,4 @@ pylint
 mypy
 bandit
 types-requests
+types-python-dateutil

tests/repository_data/generate_project_data.py → tests/repository_data/generate_project_data_old.py

@@ -17,8 +17,8 @@
   See LICENSE-MIT.txt OR LICENSE-APACHE.txt for licensing information.
 
 <Purpose>
-  Generate a pre-fabricated set of metadata files for 'test_developer_tool.py'
-  test cases.
+  Generate a pre-fabricated set of metadata files for
+  'test_developer_tool_old.py' test cases.
 """
 
 import shutil

tests/repository_simulator.py

@@ -216,7 +216,7 @@ def fetch(self, url: str) -> Iterator[bytes]:
                 role = ver_and_name
                 version = None
 
-            yield self._fetch_metadata(role, version)
+            yield self.fetch_metadata(role, version)
         elif path.startswith("/targets/"):
             # figure out target path and hash prefix
             target_path = path[len("/targets/") :]
@@ -228,11 +228,11 @@ def fetch(self, url: str) -> Iterator[bytes]:
                 prefix, _, filename = prefixed_filename.partition(".")
             target_path = f"{dir_parts}{sep}{filename}"
 
-            yield self._fetch_target(target_path, prefix)
+            yield self.fetch_target(target_path, prefix)
         else:
             raise FetcherHTTPError(f"Unknown path '{path}'", 404)
 
-    def _fetch_target(
+    def fetch_target(
         self, target_path: str, target_hash: Optional[str]
     ) -> bytes:
         """Return data for 'target_path', checking 'target_hash' if it is given.
@@ -253,9 +253,7 @@ def _fetch_target(
         logger.debug("fetched target %s", target_path)
         return repo_target.data
 
-    def _fetch_metadata(
-        self, role: str, version: Optional[int] = None
-    ) -> bytes:
+    def fetch_metadata(self, role: str, version: Optional[int] = None) -> bytes:
         """Return signed metadata for 'role', using 'version' if it is given.
 
         If version is None, non-versioned metadata is being requested.
@@ -298,7 +296,7 @@ def _fetch_metadata(
     def _compute_hashes_and_length(
         self, role: str
     ) -> Tuple[Dict[str, str], int]:
-        data = self._fetch_metadata(role)
+        data = self.fetch_metadata(role)
         digest_object = sslib_hash.digest(sslib_hash.DEFAULT_HASH_ALGORITHM)
         digest_object.update(data)
         hashes = {sslib_hash.DEFAULT_HASH_ALGORITHM: digest_object.hexdigest()}
@@ -392,12 +390,12 @@ def write(self) -> None:
 
         for ver in range(1, len(self.signed_roots) + 1):
             with open(os.path.join(dest_dir, f"{ver}.root.json"), "wb") as f:
-                f.write(self._fetch_metadata(Root.type, ver))
+                f.write(self.fetch_metadata(Root.type, ver))
 
         for role in [Timestamp.type, Snapshot.type, Targets.type]:
             with open(os.path.join(dest_dir, f"{role}.json"), "wb") as f:
-                f.write(self._fetch_metadata(role))
+                f.write(self.fetch_metadata(role))
 
         for role in self.md_delegates:
             with open(os.path.join(dest_dir, f"{role}.json"), "wb") as f:
-                f.write(self._fetch_metadata(role))
+                f.write(self.fetch_metadata(role))

tests/simple_https_server.py → tests/simple_https_server_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program>
-  simple_https_server.py
+  simple_https_server_old.py
 
 <Author>
   Vladimir Diaz.
@@ -53,7 +53,7 @@
 print(port_message)
 
 if len(sys.argv) > 1 and certfile != sys.argv[1]:
-  print('simple_https_server: cert file was not found: ' + sys.argv[1] +
+  print('simple_https_server_old: cert file was not found: ' + sys.argv[1] +
       '; using default: ' + certfile + " certfile")
 
 httpd.serve_forever()

tests/slow_retrieval_server.py → tests/slow_retrieval_server_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  slow_retrieval_server.py
+  slow_retrieval_server_old.py
 
 <Author>
   Konstantin Andrianov.
@@ -18,7 +18,7 @@
 
 <Purpose>
   Server that throttles data by sending one byte at a time (specified time
-  interval 'DELAY').  The server is used in 'test_slow_retrieval_attack.py'.
+  interval 'DELAY').  The server is used in 'test_slow_retrieval_attack_old.py'.
 """
 
 import os

tests/test_api.py

@@ -28,6 +28,7 @@
 from tests import utils
 from tuf import exceptions
 from tuf.api.metadata import (
+    TOP_LEVEL_ROLE_NAMES,
     DelegatedRole,
     Delegations,
     Key,
@@ -136,7 +137,7 @@ def test_compact_json(self) -> None:
         )
 
     def test_read_write_read_compare(self) -> None:
-        for metadata in [Root.type, Snapshot.type, Timestamp.type, Targets.type]:
+        for metadata in TOP_LEVEL_ROLE_NAMES:
             path = os.path.join(self.repo_dir, "metadata", metadata + ".json")
             md_obj = Metadata.from_file(path)
 
@@ -148,7 +149,7 @@ def test_read_write_read_compare(self) -> None:
             os.remove(path_2)
 
     def test_to_from_bytes(self) -> None:
-        for metadata in [Root.type, Snapshot.type, Timestamp.type, Targets.type]:
+        for metadata in TOP_LEVEL_ROLE_NAMES:
             path = os.path.join(self.repo_dir, "metadata", metadata + ".json")
             with open(path, "rb") as f:
                 metadata_bytes = f.read()
@@ -710,7 +711,9 @@ def test_targetfile_from_file(self) -> None:
 
     def test_targetfile_from_data(self) -> None:
         data = b"Inline test content"
-        target_file_path = os.path.join(self.repo_dir, Targets.type, "file1.txt")
+        target_file_path = os.path.join(
+            self.repo_dir, Targets.type, "file1.txt"
+        )
 
         # Test with a valid hash algorithm
         targetfile_from_data = TargetFile.from_data(

tests/test_arbitrary_package_attack.py → tests/test_arbitrary_package_attack_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_arbitrary_package_attack.py
+  test_arbitrary_package_attack_old.py
 
 <Author>
   Konstantin Andrianov.

tests/test_developer_tool.py → tests/test_developer_tool_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_developer_tool.py.
+  test_developer_tool_old.py.
 
 <Authors>
   Santiago Torres Arias <torresariass@gmail.com>

tests/test_download.py → tests/test_download_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program>
-  test_download.py
+  test_download_old.py
 
 <Author>
   Konstantin Andrianov.
@@ -19,7 +19,7 @@
 <Purpose>
   Unit test for 'download.py'.
 
-  NOTE: Make sure test_download.py is ran in 'tuf/tests/' directory.
+  NOTE: Make sure test_download_old.py is ran in 'tuf/tests/' directory.
   Otherwise, module that launches simple server would not be found.
 
   TODO: Adopt the environment variable management from test_proxy_use.py here.
@@ -274,16 +274,16 @@ def test_https_connection(self):
 
 
     good_https_server_handler = utils.TestServerProcess(log=logger,
-        server='simple_https_server.py',
+        server='simple_https_server_old.py',
         extra_cmd_args=[good_cert_fname])
     good2_https_server_handler = utils.TestServerProcess(log=logger,
-        server='simple_https_server.py',
+        server='simple_https_server_old.py',
         extra_cmd_args=[good2_cert_fname])
     bad_https_server_handler = utils.TestServerProcess(log=logger,
-        server='simple_https_server.py',
+        server='simple_https_server_old.py',
         extra_cmd_args=[bad_cert_fname])
     expd_https_server_handler = utils.TestServerProcess(log=logger,
-        server='simple_https_server.py',
+        server='simple_https_server_old.py',
         extra_cmd_args=[expired_cert_fname])
 
     suffix = '/' +  os.path.basename(target_filepath)

tests/test_endless_data_attack.py → tests/test_endless_data_attack_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_endless_data_attack.py
+  test_endless_data_attack_old.py
 
 <Author>
   Konstantin Andrianov.

tests/test_examples.py

@@ -7,9 +7,13 @@
 import glob
 import os
 import shutil
+import sys
 import tempfile
 import unittest
 from pathlib import Path
+from typing import ClassVar, List
+
+from tests import utils
 
 
 class TestRepoExamples(unittest.TestCase):
@@ -20,26 +24,30 @@ class TestRepoExamples(unittest.TestCase):
 
     """
 
+    repo_examples_dir: ClassVar[Path]
+
     @classmethod
-    def setUpClass(cls):
+    def setUpClass(cls) -> None:
         """Locate and cache 'repo_example' dir."""
         base = Path(__file__).resolve().parents[1]
         cls.repo_examples_dir = base / "examples" / "repo_example"
 
-    def setUp(self):
+    def setUp(self) -> None:
         """Create and change into test dir.
         NOTE: Test scripts are expected to create dirs/files in new CWD."""
         self.original_cwd = os.getcwd()
         self.base_test_dir = os.path.realpath(tempfile.mkdtemp())
         os.chdir(self.base_test_dir)
 
-    def tearDown(self):
+    def tearDown(self) -> None:
         """Change back to original dir and remove test dir, which may contain
         dirs/files the test created at test-time CWD."""
         os.chdir(self.original_cwd)
         shutil.rmtree(self.base_test_dir)
 
-    def _run_script_and_assert_files(self, script_name, filenames_created):
+    def _run_script_and_assert_files(
+        self, script_name: str, filenames_created: List[str]
+    ) -> None:
         """Run script in 'repo_example' dir and assert that it created the
         files corresponding to the passed filenames inside a 'tmp*' test dir at
         CWD."""
@@ -63,7 +71,7 @@ def _run_script_and_assert_files(self, script_name, filenames_created):
                 metadata_path.exists(), f"missing '{metadata_path}' file"
             )
 
-    def test_basic_repo(self):
+    def test_basic_repo(self) -> None:
         """Run 'basic_repo.py' and assert creation of metadata files."""
         self._run_script_and_assert_files(
             "basic_repo.py",
@@ -81,4 +89,5 @@ def test_basic_repo(self):
 
 
 if __name__ == "__main__":
+    utils.configure_test_logging(sys.argv)
     unittest.main()

tests/test_extraneous_dependencies_attack.py → tests/test_extraneous_dependencies_attack_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_extraneous_dependencies_attack.py
+  test_extraneous_dependencies_attack_old.py
 
 <Author>
   Zane Fisher.

tests/test_formats.py → tests/test_formats_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_formats.py
+  test_formats_old.py
 
 <Author>
   Vladimir Diaz <vladimir.v.diaz@gmail.com>

tests/test_indefinite_freeze_attack.py → tests/test_indefinite_freeze_attack_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_indefinite_freeze_attack.py
+  test_indefinite_freeze_attack_old.py
 
 <Author>
   Konstantin Andrianov.
@@ -36,7 +36,6 @@
   metadata without the client being aware.
 """
 
-import datetime
 import os
 import time
 import tempfile

tests/test_key_revocation_integration.py → tests/test_key_revocation_integration_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_key_revocation_integration.py
+  test_key_revocation_integration_old.py
 
 <Author>
   Vladimir Diaz.
@@ -18,7 +18,7 @@
 
 <Purpose>
   Integration test that verifies top-level roles are updated after all of their
-  keys have been revoked. There are unit tests in 'test_repository_tool.py'
+  keys have been revoked. There are unit tests in 'test_repository_tool_old.py'
   that verify key and role revocation of specific roles, but these should be
   expanded to verify key revocations over the span of multiple snapshots of the
   repository.

tests/test_keydb.py → tests/test_keydb_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_keydb.py
+  test_keydb_old.py
 
 <Author>
   Vladimir Diaz <vladimir.v.diaz@gmail.com>

tests/test_log.py → tests/test_log_old.py

@@ -5,7 +5,7 @@
 
 """
 <Program Name>
-  test_log.py
+  test_log_old.py
 
 <Authors>
   Vladimir Diaz <vladimir.v.diaz@gmail.com>