From 5cf5b6049f4e4cc83b08c1b2f36aaf0ad48b67f1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 28 Feb 2024 20:48:52 -0500 Subject: [PATCH] Revise test suite docstrings and comments In a more limited way than in the git/ code. Docstring revisions are, in spirit, along the same lines as those in the git package, but much more conservative, because the tests' docstrings are not rendered into Sphinx documentation and there is no current plan to do so (so the tradeoffs are different, with even a tiny decrease in clarity when read in the code being a reason to avoid changes that use Sphinx roles more robustly or that would improve hypothetical rendered documentation), and because the test suite uses docstrings more sparingly and the existing docstrings were mostly already clear and easy to read. This wraps commnts to 88 columns as most comments now are in the git package, except it avoids doing so when doing so would make anything even slightly less clear, and where it would require significant further style or spacing changes for it to remain obvious (even before reading a comment) what the comment applies to, and in most portions of tutorial-generating test case code (where, although clarity would improve when reading the tests, it might sometimes decrease in the generated documentation). --- test/lib/helper.py | 13 ++-- test/performance/test_streams.py | 4 +- test/test_base.py | 4 +- test/test_clone.py | 4 +- test/test_commit.py | 16 +++-- test/test_config.py | 12 ++-- test/test_diff.py | 23 ++++--- test/test_docs.py | 26 ++++---- test/test_fun.py | 3 +- test/test_git.py | 22 +++--- test/test_index.py | 49 +++++++------- test/test_installation.py | 6 +- test/test_refs.py | 50 +++++++++----- test/test_remote.py | 39 +++++------ test/test_repo.py | 34 +++++----- test/test_submodule.py | 111 ++++++++++++++++++------------- test/test_util.py | 13 ++-- 17 files changed, 243 insertions(+), 186 deletions(-) diff --git a/test/lib/helper.py b/test/lib/helper.py index f951a6a12..26469ed5d 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -346,17 +346,20 @@ class TestBase(TestCase): """Base class providing default functionality to all tests such as: - Utility functions provided by the TestCase base of the unittest method such as:: + self.fail("todo") self.assertRaises(...) - Class level repository which is considered read-only as it is shared among all test cases in your type. + Access it using:: - self.rorepo # 'ro' stands for read-only + + self.rorepo # 'ro' stands for read-only The rorepo is in fact your current project's git repo. If you refer to specific - shas for your objects, be sure you choose some that are part of the immutable portion - of the project history (so that tests don't fail for others). + shas for your objects, be sure you choose some that are part of the immutable + portion of the project history (so that tests don't fail for others). """ def _small_repo_url(self): @@ -383,8 +386,8 @@ def tearDownClass(cls): def _make_file(self, rela_path, data, repo=None): """ - Create a file at the given path relative to our repository, filled - with the given data. + Create a file at the given path relative to our repository, filled with the + given data. :return: An absolute path to the created file. """ diff --git a/test/performance/test_streams.py b/test/performance/test_streams.py index ba5cbe415..56b5274ec 100644 --- a/test/performance/test_streams.py +++ b/test/performance/test_streams.py @@ -25,8 +25,8 @@ class TestObjDBPerformance(TestBigRepoR): @with_rw_repo("HEAD", bare=True) def test_large_data_streaming(self, rwrepo): - # TODO: This part overlaps with the same file in gitdb.test.performance.test_stream. - # It should be shared if possible. + # TODO: This part overlaps with the same file in + # gitdb.test.performance.test_stream. It should be shared if possible. ldb = LooseObjectDB(osp.join(rwrepo.git_dir, "objects")) for randomize in range(2): diff --git a/test/test_base.py b/test/test_base.py index 693c97f20..ef7486e86 100644 --- a/test/test_base.py +++ b/test/test_base.py @@ -135,8 +135,8 @@ def test_add_unicode(self, rw_repo): # https://github.com/gitpython-developers/GitPython/issues/147#issuecomment-68881897 # Therefore, it must be added using the Python implementation. rw_repo.index.add([file_path]) - # However, when the test winds down, rmtree fails to delete this file, which is recognized - # as ??? only. + # However, when the test winds down, rmtree fails to delete this file, which + # is recognized as ??? only. else: # On POSIX, we can just add Unicode files without problems. rw_repo.git.add(rw_repo.working_dir) diff --git a/test/test_clone.py b/test/test_clone.py index dcab7ad6f..be2e6b19b 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -19,8 +19,8 @@ def test_checkout_in_non_empty_dir(self, rw_dir): garbage_file = non_empty_dir / "not-empty" garbage_file.write_text("Garbage!") - # Verify that cloning into the non-empty dir fails while complaining about - # the target directory not being empty/non-existent. + # Verify that cloning into the non-empty dir fails while complaining about the + # target directory not being empty/non-existent. try: self.rorepo.clone(non_empty_dir) except git.GitCommandError as exc: diff --git a/test/test_commit.py b/test/test_commit.py index fddb91c17..5571b9ecb 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -27,8 +27,8 @@ class TestCommitSerialization(TestBase): def assert_commit_serialization(self, rwrepo, commit_id, print_performance_info=False): - """Traverse all commits in the history of commit identified by commit_id and check - if the serialization works. + """Traverse all commits in the history of commit identified by commit_id and + check if the serialization works. :param print_performance_info: If True, we will show how fast we are. """ @@ -317,8 +317,9 @@ def test_count(self): self.assertEqual(self.rorepo.tag("refs/tags/0.1.5").commit.count(), 143) def test_list(self): - # This doesn't work anymore, as we will either attempt getattr with bytes, or compare 20 byte string - # with actual 20 byte bytes. This usage makes no sense anyway. + # This doesn't work anymore, as we will either attempt getattr with bytes, or + # compare 20 byte string with actual 20 byte bytes. This usage makes no sense + # anyway. assert isinstance( Commit.list_items(self.rorepo, "0.1.5", max_count=5)["5117c9c8a4d3af19a9958677e45cda9269de1541"], Commit, @@ -383,8 +384,8 @@ def test_serialization_unicode_support(self): self.assertEqual(cmt.author.name, ncmt.author.name) self.assertEqual(cmt.message, ncmt.message) - # Actually, it can't be printed in a shell as repr wants to have ascii only - # it appears. + # Actually, it can't be printed in a shell as repr wants to have ascii only it + # appears. cmt.author.__repr__() def test_invalid_commit(self): @@ -498,7 +499,8 @@ def test_trailers(self): KEY_2 = "Key" VALUE_2 = "Value with inner spaces" - # Check that the following trailer example is extracted from multiple msg variations. + # Check that the following trailer example is extracted from multiple msg + # variations. TRAILER = f"{KEY_1}: {VALUE_1_1}\n{KEY_2}: {VALUE_2}\n{KEY_1}: {VALUE_1_2}" msgs = [ f"Subject\n\n{TRAILER}\n", diff --git a/test/test_config.py b/test/test_config.py index f493c5672..4843d91eb 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -41,7 +41,7 @@ def _to_memcache(self, file_path): return sio def test_read_write(self): - # writer must create the exact same file as the one read before + # The writer must create the exact same file as the one read before. for filename in ("git_config", "git_config_global"): file_obj = self._to_memcache(fixture_path(filename)) with GitConfigParser(file_obj, read_only=False) as w_config: @@ -56,7 +56,8 @@ def test_read_write(self): self._to_memcache(fixture_path(filename)).getvalue(), ) - # Creating an additional config writer must fail due to exclusive access. + # Creating an additional config writer must fail due to exclusive + # access. with self.assertRaises(IOError): GitConfigParser(file_obj, read_only=False) @@ -91,8 +92,8 @@ def test_includes_order(self): r_config.read() # Enforce reading. # Simple inclusions, again checking them taking precedence. assert r_config.get_value("sec", "var0") == "value0_included" - # This one should take the git_config_global value since included - # values must be considered as soon as they get them. + # This one should take the git_config_global value since included values + # must be considered as soon as they get them. assert r_config.get_value("diff", "tool") == "meld" try: # FIXME: Split this assertion out somehow and mark it xfail (or fix it). @@ -109,7 +110,8 @@ def test_lock_reentry(self, rw_dir): # Entering again locks the file again... with gcp as cw: cw.set_value("include", "some_other_value", "b") - # ...so creating an additional config writer must fail due to exclusive access. + # ...so creating an additional config writer must fail due to exclusive + # access. with self.assertRaises(IOError): GitConfigParser(fpl, read_only=False) # but work when the lock is removed diff --git a/test/test_diff.py b/test/test_diff.py index 87f92f5d1..cdd473f7f 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -271,18 +271,18 @@ def test_diff_unsafe_paths(self): self.assertEqual(res[10].b_rawpath, b"path/\x80-invalid-unicode-path.txt") # The "Moves" - # NOTE: The path prefixes a/ and b/ here are legit! We're actually - # verifying that it's not "a/a/" that shows up, see the fixture data. - self.assertEqual(res[11].a_path, "a/with spaces") # NOTE: path a/ here legit! - self.assertEqual(res[11].b_path, "b/with some spaces") # NOTE: path b/ here legit! + # NOTE: The path prefixes "a/" and "b/" here are legit! We're actually verifying + # that it's not "a/a/" that shows up; see the fixture data. + self.assertEqual(res[11].a_path, "a/with spaces") # NOTE: path "a/"" legit! + self.assertEqual(res[11].b_path, "b/with some spaces") # NOTE: path "b/"" legit! self.assertEqual(res[12].a_path, "a/ending in a space ") self.assertEqual(res[12].b_path, "b/ending with space ") self.assertEqual(res[13].a_path, 'a/"with-quotes"') self.assertEqual(res[13].b_path, 'b/"with even more quotes"') def test_diff_patch_format(self): - # Test all of the 'old' format diffs for completeness - it should at least - # be able to deal with it. + # Test all of the 'old' format diffs for completeness - it should at least be + # able to deal with it. fixtures = ( "diff_2", "diff_2f", @@ -345,8 +345,9 @@ def test_diff_submodule(self): repo.create_tag("2") diff = repo.commit("1").diff(repo.commit("2"))[0] - # If diff is unable to find the commit hashes (looks in wrong repo) the *_blob.size - # property will be a string containing exception text, an int indicates success. + # If diff is unable to find the commit hashes (looks in wrong repo) the + # *_blob.size property will be a string containing exception text, an int + # indicates success. self.assertIsInstance(diff.a_blob.size, int) self.assertIsInstance(diff.b_blob.size, int) @@ -392,9 +393,9 @@ def test_diff_interface(self): # END for each other side # END for each commit - # Assert that we could always find at least one instance of the members we - # can iterate in the diff index - if not this indicates its not working correctly - # or our test does not span the whole range of possibilities. + # Assert that we could always find at least one instance of the members we can + # iterate in the diff index - if not this indicates its not working correctly or + # our test does not span the whole range of possibilities. for key, value in assertion_map.items(): self.assertIsNotNone(value, "Did not find diff for %s" % key) # END for each iteration type diff --git a/test/test_docs.py b/test/test_docs.py index 48922eba8..409f66bb3 100644 --- a/test/test_docs.py +++ b/test/test_docs.py @@ -19,8 +19,9 @@ class Tutorials(TestBase): def tearDown(self): gc.collect() - # ACTUALLY skipped by git.util.rmtree (in local onerror function), from the last call to it via - # git.objects.submodule.base.Submodule.remove (at "handle separate bare repository"), line 1062. + # ACTUALLY skipped by git.util.rmtree (in local onerror function), from the last + # call to it via git.objects.submodule.base.Submodule.remove + # (at "handle separate bare repository"), line 1062. # # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, # "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: " @@ -31,8 +32,8 @@ def test_init_repo_object(self, rw_dir): from git import Repo # rorepo is a Repo instance pointing to the git-python repository. - # For all you know, the first argument to Repo is a path to the repository - # you want to work with. + # For all you know, the first argument to Repo is a path to the repository you + # want to work with. repo = Repo(self.rorepo.working_tree_dir) assert not repo.bare # ![1-test_init_repo_object] @@ -149,8 +150,8 @@ def update(self, op_code, cur_count, max_count=None, message=""): assert origin.exists() for fetch_info in origin.fetch(progress=MyProgressPrinter()): print("Updated %s to %s" % (fetch_info.ref, fetch_info.commit)) - # Create a local branch at the latest fetched master. We specify the name statically, but you have all - # information to do it programmatically as well. + # Create a local branch at the latest fetched master. We specify the name + # statically, but you have all information to do it programmatically as well. bare_master = bare_repo.create_head("master", origin.refs.master) bare_repo.head.set_reference(bare_master) assert not bare_repo.delete_remote(origin).exists() @@ -188,9 +189,9 @@ def update(self, op_code, cur_count, max_count=None, message=""): # submodules # [14-test_init_repo_object] - # Create a new submodule and check it out on the spot, setup to track master branch of `bare_repo`. - # As our GitPython repository has submodules already that point to GitHub, make sure we don't - # interact with them. + # Create a new submodule and check it out on the spot, setup to track master + # branch of `bare_repo`. As our GitPython repository has submodules already that + # point to GitHub, make sure we don't interact with them. for sm in cloned_repo.submodules: assert not sm.remove().exists() # after removal, the sm doesn't exist anymore sm = cloned_repo.create_submodule("mysubrepo", "path/to/subrepo", url=bare_repo.git_dir, branch="master") @@ -424,8 +425,8 @@ def test_references_and_objects(self, rw_dir): with origin.config_writer as cw: cw.set("pushurl", "other_url") - # Please note that in Python 2, writing origin.config_writer.set(...) is totally safe. - # In py3 __del__ calls can be delayed, thus not writing changes in time. + # Please note that in Python 2, writing origin.config_writer.set(...) is totally + # safe. In py3 __del__ calls can be delayed, thus not writing changes in time. # ![26-test_references_and_objects] # [27-test_references_and_objects] @@ -462,7 +463,8 @@ def test_references_and_objects(self, rw_dir): # ![29-test_references_and_objects] # [30-test_references_and_objects] - # Check out the branch using git-checkout. It will fail as the working tree appears dirty. + # Check out the branch using git-checkout. + # It will fail as the working tree appears dirty. self.assertRaises(git.GitCommandError, repo.heads.master.checkout) repo.heads.past_branch.checkout() # ![30-test_references_and_objects] diff --git a/test/test_fun.py b/test/test_fun.py index 566bc9aae..2d30d355a 100644 --- a/test/test_fun.py +++ b/test/test_fun.py @@ -35,7 +35,8 @@ def _assert_index_entries(self, entries, trees): # END assert entry matches fully def test_aggressive_tree_merge(self): - # Head tree with additions, removals and modification compared to its predecessor. + # Head tree with additions, removals and modification compared to its + # predecessor. odb = self.rorepo.odb HC = self.rorepo.commit("6c1faef799095f3990e9970bc2cb10aa0221cf9c") H = HC.tree diff --git a/test/test_git.py b/test/test_git.py index 97e21cad4..e1a8bda5e 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -253,21 +253,27 @@ def test_it_avoids_upcasing_unrelated_environment_variable_names(self): if old_name == old_name.upper(): raise RuntimeError("test bug or strange locale: old_name invariant under upcasing") - # Step 1: Set the environment variable in this parent process. Because os.putenv is a thin - # wrapper around a system API, os.environ never sees the variable in this parent - # process, so the name is not upcased even on Windows. + # Step 1 + # + # Set the environment variable in this parent process. Because os.putenv is a + # thin wrapper around a system API, os.environ never sees the variable in this + # parent process, so the name is not upcased even on Windows. os.putenv(old_name, "1") - # Step 2: Create the child process that inherits the environment variable. The child uses - # GitPython, and we are testing that it passes the variable with the exact original - # name to its own child process (the grandchild). + # Step 2 + # + # Create the child process that inherits the environment variable. The child + # uses GitPython, and we are testing that it passes the variable with the exact + # original name to its own child process (the grandchild). cmdline = [ sys.executable, fixture_path("env_case.py"), # Contains steps 3 and 4. self.rorepo.working_dir, old_name, ] - pair_text = subprocess.check_output(cmdline, shell=False, text=True) # Run steps 3 and 4. + + # Run steps 3 and 4. + pair_text = subprocess.check_output(cmdline, shell=False, text=True) new_name = pair_text.split("=")[0] self.assertEqual(new_name, old_name) @@ -668,7 +674,7 @@ def test_successful_default_refresh_invalidates_cached_version_info(self): # as unintended shell expansions can occur, and is deprecated. Instead, # use a custom command, by setting the GIT_PYTHON_GIT_EXECUTABLE # environment variable to git.cmd or by passing git.cmd's full path to - # git.refresh. Or wrap the script with a .exe shim. + # git.refresh. Or wrap the script with a .exe shim.) stack.enter_context(mock.patch.object(Git, "USE_SHELL", True)) new_git = Git() diff --git a/test/test_index.py b/test/test_index.py index fd62bb893..fa64b82a2 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -219,8 +219,7 @@ def _fprogress(self, path, done, item): self._fprogress_map[path] = curval + 1 def _fprogress_add(self, path, done, item): - """Called as progress func - we keep track of the proper - call order""" + """Called as progress func - we keep track of the proper call order.""" assert item is not None self._fprogress(path, done, item) @@ -385,17 +384,17 @@ def test_index_merge_tree(self, rw_repo): # FAKE MERGE ############# - # Add a change with a NULL sha that should conflict with next_commit. We - # pretend there was a change, but we do not even bother adding a proper - # sha for it (which makes things faster of course). + # Add a change with a NULL sha that should conflict with next_commit. We pretend + # there was a change, but we do not even bother adding a proper sha for it + # (which makes things faster of course). manifest_fake_entry = BaseIndexEntry((manifest_entry[0], b"\0" * 20, 0, manifest_entry[3])) # Try write flag. self._assert_entries(rw_repo.index.add([manifest_fake_entry], write=False)) - # Add actually resolves the null-hex-sha for us as a feature, but we can - # edit the index manually. + # Add actually resolves the null-hex-sha for us as a feature, but we can edit + # the index manually. assert rw_repo.index.entries[manifest_key].binsha != Object.NULL_BIN_SHA - # We must operate on the same index for this! It's a bit problematic as - # it might confuse people. + # We must operate on the same index for this! It's a bit problematic as it might + # confuse people. index = rw_repo.index index.entries[manifest_key] = IndexEntry.from_base(manifest_fake_entry) index.write() @@ -404,19 +403,20 @@ def test_index_merge_tree(self, rw_repo): # Write an unchanged index (just for the fun of it). rw_repo.index.write() - # A three way merge would result in a conflict and fails as the command will - # not overwrite any entries in our index and hence leave them unmerged. This is + # A three way merge would result in a conflict and fails as the command will not + # overwrite any entries in our index and hence leave them unmerged. This is # mainly a protection feature as the current index is not yet in a tree. self.assertRaises(GitCommandError, index.merge_tree, next_commit, base=parent_commit) - # The only way to get the merged entries is to safe the current index away into a tree, - # which is like a temporary commit for us. This fails as well as the NULL sha does not - # have a corresponding object. + # The only way to get the merged entries is to safe the current index away into + # a tree, which is like a temporary commit for us. This fails as well as the + # NULL sha does not have a corresponding object. # NOTE: missing_ok is not a kwarg anymore, missing_ok is always true. # self.assertRaises(GitCommandError, index.write_tree) - # If missing objects are okay, this would work though (they are always okay now). - # As we can't read back the tree with NULL_SHA, we rather set it to something else. + # If missing objects are okay, this would work though (they are always okay + # now). As we can't read back the tree with NULL_SHA, we rather set it to + # something else. index.entries[manifest_key] = IndexEntry(manifest_entry[:1] + (hex_to_bin("f" * 40),) + manifest_entry[2:]) tree = index.write_tree() @@ -428,7 +428,7 @@ def test_index_merge_tree(self, rw_repo): @with_rw_repo("0.1.6") def test_index_file_diffing(self, rw_repo): - # Default Index instance points to our index. + # Default IndexFile instance points to our index. index = IndexFile(rw_repo) assert index.path is not None assert len(index.entries) @@ -439,8 +439,8 @@ def test_index_file_diffing(self, rw_repo): # Could sha it, or check stats. # Test diff. - # Resetting the head will leave the index in a different state, and the - # diff will yield a few changes. + # Resetting the head will leave the index in a different state, and the diff + # will yield a few changes. cur_head_commit = rw_repo.head.reference.commit rw_repo.head.reset("HEAD~6", index=True, working_tree=False) @@ -956,10 +956,10 @@ def test_index_new(self): @with_rw_repo("HEAD", bare=True) def test_index_bare_add(self, rw_bare_repo): - # Something is wrong after cloning to a bare repo, reading the - # property rw_bare_repo.working_tree_dir will return '/tmp' - # instead of throwing the Exception we are expecting. This is - # a quick hack to make this test fail when expected. + # Something is wrong after cloning to a bare repo, reading the property + # rw_bare_repo.working_tree_dir will return '/tmp' instead of throwing the + # Exception we are expecting. This is a quick hack to make this test fail when + # expected. assert rw_bare_repo.working_tree_dir is None assert rw_bare_repo.bare contents = b"This is a BytesIO file" @@ -984,7 +984,8 @@ def test_index_bare_add(self, rw_bare_repo): @with_rw_directory def test_add_utf8P_path(self, rw_dir): - # NOTE: fp is not a Unicode object in Python 2 (which is the source of the problem). + # NOTE: fp is not a Unicode object in Python 2 + # (which is the source of the problem). fp = osp.join(rw_dir, "ø.txt") with open(fp, "wb") as fs: fs.write("content of ø".encode("utf-8")) diff --git a/test/test_installation.py b/test/test_installation.py index 15ed5b13b..ae6472e98 100644 --- a/test/test_installation.py +++ b/test/test_installation.py @@ -46,9 +46,9 @@ def test_installation(self, rw_dir): msg=result.stderr or result.stdout or "Dependencies not installed", ) - # Even IF gitdb or any other dependency is supplied during development - # by inserting its location into PYTHONPATH or otherwise patched into - # sys.path, make sure it is not wrongly inserted as the *first* entry. + # Even IF gitdb or any other dependency is supplied during development by + # inserting its location into PYTHONPATH or otherwise patched into sys.path, + # make sure it is not wrongly inserted as the *first* entry. result = subprocess.run( [venv.python, "-c", "import sys; import git; print(sys.path)"], stdout=subprocess.PIPE, diff --git a/test/test_refs.py b/test/test_refs.py index 2656ceab1..28db70c6e 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -245,8 +245,8 @@ def test_head_reset(self, rw_repo): cur_head.reset(new_head_commit) rw_repo.index.checkout(["lib"], force=True) - # Now that we have a write write repo, change the HEAD reference - it's - # like "git-reset --soft". + # Now that we have a write write repo, change the HEAD reference - it's like + # "git-reset --soft". heads = rw_repo.heads assert heads for head in heads: @@ -349,8 +349,8 @@ def test_head_reset(self, rw_repo): for remote in remotes: refs = remote.refs - # If a HEAD exists, it must be deleted first. Otherwise it might - # end up pointing to an invalid ref it the ref was deleted before. + # If a HEAD exists, it must be deleted first. Otherwise it might end up + # pointing to an invalid ref it the ref was deleted before. remote_head_name = "HEAD" if remote_head_name in refs: RemoteReference.delete(rw_repo, refs[remote_head_name]) @@ -383,7 +383,7 @@ def test_head_reset(self, rw_repo): # Setting a non-commit as commit fails, but succeeds as object. head_tree = head.commit.tree self.assertRaises(ValueError, setattr, head, "commit", head_tree) - assert head.commit == old_commit # and the ref did not change + assert head.commit == old_commit # And the ref did not change. # We allow heads to point to any object. head.object = head_tree assert head.object == head_tree @@ -492,8 +492,8 @@ def test_head_reset(self, rw_repo): # Would raise if the symref wouldn't have been deleted (probably). symref = SymbolicReference.create(rw_repo, symref_path, cur_head.reference) - # Test symbolic references which are not at default locations like HEAD - # or FETCH_HEAD - they may also be at spots in refs of course. + # Test symbolic references which are not at default locations like HEAD or + # FETCH_HEAD - they may also be at spots in refs of course. symbol_ref_path = "refs/symbol_ref" symref = SymbolicReference(rw_repo, symbol_ref_path) assert symref.path == symbol_ref_path @@ -525,14 +525,13 @@ def test_head_reset(self, rw_repo): assert active_branch in heads assert rw_repo.tags - # We should be able to iterate all symbolic refs as well - in that case - # we should expect only symbolic references to be returned. + # We should be able to iterate all symbolic refs as well - in that case we + # should expect only symbolic references to be returned. for symref in SymbolicReference.iter_items(rw_repo): assert not symref.is_detached - # When iterating references, we can get references and symrefs - # when deleting all refs, I'd expect them to be gone! Even from - # the packed ones. + # When iterating references, we can get references and symrefs when deleting all + # refs, I'd expect them to be gone! Even from the packed ones. # For this to work, we must not be on any branch. rw_repo.head.reference = rw_repo.head.commit deleted_refs = set() @@ -577,9 +576,9 @@ def test_head_reset(self, rw_repo): self.assertRaises(ValueError, setattr, ref, "commit", "nonsense") assert not ref.is_valid() - # I am sure I had my reason to make it a class method at first, but - # now it doesn't make so much sense anymore, want an instance method as well - # See http://byronimo.lighthouseapp.com/projects/51787-gitpython/tickets/27 + # I am sure I had my reason to make it a class method at first, but now it + # doesn't make so much sense anymore, want an instance method as well. See: + # http://byronimo.lighthouseapp.com/projects/51787-gitpython/tickets/27 Reference.delete(ref.repo, ref.path) assert not ref.is_valid() @@ -619,8 +618,8 @@ def test_reflog(self): def test_refs_outside_repo(self): # Create a file containing a valid reference outside the repository. Attempting - # to access it should raise an exception, due to it containing a parent directory - # reference ('..'). This tests for CVE-2023-41040. + # to access it should raise an exception, due to it containing a parent + # directory reference ('..'). This tests for CVE-2023-41040. git_dir = Path(self.rorepo.git_dir) repo_parent_dir = git_dir.parent.parent with tempfile.NamedTemporaryFile(dir=repo_parent_dir) as ref_file: @@ -630,37 +629,52 @@ def test_refs_outside_repo(self): self.assertRaises(BadName, self.rorepo.commit, f"../../{ref_file_name}") def test_validity_ref_names(self): + """Ensure ref names are checked for validity. + + This is based on the rules specified in: + https://git-scm.com/docs/git-check-ref-format/#_description + """ check_ref = SymbolicReference._check_ref_name_valid - # Based on the rules specified in https://git-scm.com/docs/git-check-ref-format/#_description. + # Rule 1 self.assertRaises(ValueError, check_ref, ".ref/begins/with/dot") self.assertRaises(ValueError, check_ref, "ref/component/.begins/with/dot") self.assertRaises(ValueError, check_ref, "ref/ends/with/a.lock") self.assertRaises(ValueError, check_ref, "ref/component/ends.lock/with/period_lock") + # Rule 2 check_ref("valid_one_level_refname") + # Rule 3 self.assertRaises(ValueError, check_ref, "ref/contains/../double/period") + # Rule 4 for c in " ~^:": self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{c}/character") for code in range(0, 32): self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{chr(code)}/ASCII/control_character") self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{chr(127)}/ASCII/control_character") + # Rule 5 for c in "*?[": self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{c}/character") + # Rule 6 self.assertRaises(ValueError, check_ref, "/ref/begins/with/slash") self.assertRaises(ValueError, check_ref, "ref/ends/with/slash/") self.assertRaises(ValueError, check_ref, "ref/contains//double/slash/") + # Rule 7 self.assertRaises(ValueError, check_ref, "ref/ends/with/dot.") + # Rule 8 self.assertRaises(ValueError, check_ref, "ref/contains@{/at_brace") + # Rule 9 self.assertRaises(ValueError, check_ref, "@") + # Rule 10 self.assertRaises(ValueError, check_ref, "ref/contain\\s/backslash") + # Valid reference name should not raise. check_ref("valid/ref/name") diff --git a/test/test_remote.py b/test/test_remote.py index df6034326..c0bd11f80 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -294,11 +294,11 @@ def get_info(res, remote, name): # Provoke to receive actual objects to see what kind of output we have to # expect. For that we need a remote transport protocol. - # Create a new UN-shared repo and fetch into it after we pushed a change - # to the shared repo. + # Create a new UN-shared repo and fetch into it after we pushed a change to the + # shared repo. other_repo_dir = tempfile.mktemp("other_repo") - # Must clone with a local path for the repo implementation not to freak out - # as it wants local paths only (which I can understand). + # Must clone with a local path for the repo implementation not to freak out as + # it wants local paths only (which I can understand). other_repo = remote_repo.clone(other_repo_dir, shared=False) remote_repo_url = osp.basename(remote_repo.git_dir) # git-daemon runs with appropriate `--base-path`. remote_repo_url = Git.polish_url("git://localhost:%s/%s" % (GIT_DAEMON_PORT, remote_repo_url)) @@ -317,10 +317,10 @@ def get_info(res, remote, name): self._commit_random_file(rw_repo) remote.push(rw_repo.head.reference) - # Here I would expect to see remote-information about packing - # objects and so on. Unfortunately, this does not happen - # if we are redirecting the output - git explicitly checks for this - # and only provides progress information to ttys. + # Here I would expect to see remote-information about packing objects and so + # on. Unfortunately, this does not happen if we are redirecting the output - + # git explicitly checks for this and only provides progress information to + # ttys. res = fetch_and_test(other_origin) finally: rmtree(other_repo_dir) @@ -333,8 +333,8 @@ def _assert_push_and_pull(self, remote, rw_repo, remote_repo): try: lhead.reference = rw_repo.heads.master except AttributeError: - # If the author is on a non-master branch, the clones might not have - # a local master yet. We simply create it. + # If the author is on a non-master branch, the clones might not have a local + # master yet. We simply create it. lhead.reference = rw_repo.create_head("master") # END master handling lhead.reset(remote.refs.master, working_tree=True) @@ -488,8 +488,8 @@ def test_base(self, rw_repo, remote_repo): self._assert_push_and_pull(remote, rw_repo, remote_repo) # FETCH TESTING - # Only for remotes - local cases are the same or less complicated - # as additional progress information will never be emitted. + # Only for remotes - local cases are the same or less complicated as + # additional progress information will never be emitted. if remote.name == "daemon_origin": self._do_test_fetch(remote, rw_repo, remote_repo, kill_after_timeout=10.0) ran_fetch_test = True @@ -508,7 +508,8 @@ def test_base(self, rw_repo, remote_repo): # Verify we can handle prunes when fetching. # stderr lines look like this: x [deleted] (none) -> origin/experiment-2012 # These should just be skipped. - # If we don't have a manual checkout, we can't actually assume there are any non-master branches. + # If we don't have a manual checkout, we can't actually assume there are any + # non-master branches. remote_repo.create_head("myone_for_deletion") # Get the branch - to be pruned later origin.fetch() @@ -812,8 +813,8 @@ def test_fetch_unsafe_url_allowed(self, rw_repo): "fd::17/foo", ] for url in urls: - # The URL will be allowed into the command, but the command will - # fail since we don't have that protocol enabled in the Git config file. + # The URL will be allowed into the command, but the command will fail + # since we don't have that protocol enabled in the Git config file. with self.assertRaises(GitCommandError): remote.fetch(url, allow_unsafe_protocols=True) assert not tmp_file.exists() @@ -880,8 +881,8 @@ def test_pull_unsafe_url_allowed(self, rw_repo): "fd::17/foo", ] for url in urls: - # The URL will be allowed into the command, but the command will - # fail since we don't have that protocol enabled in the Git config file. + # The URL will be allowed into the command, but the command will fail + # since we don't have that protocol enabled in the Git config file. with self.assertRaises(GitCommandError): remote.pull(url, allow_unsafe_protocols=True) assert not tmp_file.exists() @@ -948,8 +949,8 @@ def test_push_unsafe_url_allowed(self, rw_repo): "fd::17/foo", ] for url in urls: - # The URL will be allowed into the command, but the command will - # fail since we don't have that protocol enabled in the Git config file. + # The URL will be allowed into the command, but the command will fail + # since we don't have that protocol enabled in the Git config file. with self.assertRaises(GitCommandError): remote.push(url, allow_unsafe_protocols=True) assert not tmp_file.exists() diff --git a/test/test_repo.py b/test/test_repo.py index 465bb2574..30a44b6c1 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -543,8 +543,8 @@ def test_init(self): try: rmtree(clone_path) except OSError: - # When relative paths are used, the clone may actually be inside - # of the parent directory. + # When relative paths are used, the clone may actually be inside of + # the parent directory. pass # END exception handling @@ -556,8 +556,8 @@ def test_init(self): try: rmtree(clone_path) except OSError: - # When relative paths are used, the clone may actually be inside - # of the parent directory. + # When relative paths are used, the clone may actually be inside of + # the parent directory. pass # END exception handling @@ -832,8 +832,8 @@ def test_config_level_paths(self): assert self.rorepo._get_config_path(config_level) def test_creation_deletion(self): - # Just a very quick test to assure it generally works. There are - # specialized cases in the test_refs module. + # Just a very quick test to assure it generally works. There are specialized + # cases in the test_refs module. head = self.rorepo.create_head("new_head", "HEAD~1") self.rorepo.delete_head(head) @@ -1027,7 +1027,8 @@ def test_rev_parse(self): num_resolved += 1 except (BadName, BadObject): print("failed on %s" % path_section) - # This is fine if we have something like 112, which belongs to remotes/rname/merge-requests/112. + # This is fine if we have something like 112, which belongs to + # remotes/rname/merge-requests/112. # END exception handling # END for each token if ref_no == 3 - 1: @@ -1149,7 +1150,7 @@ def test_submodule_update(self, rwrepo): ) self.assertIsInstance(sm, Submodule) - # NOTE: the rest of this functionality is tested in test_submodule. + # NOTE: The rest of this functionality is tested in test_submodule. @with_rw_repo("HEAD") def test_git_file(self, rwrepo): @@ -1178,8 +1179,9 @@ def last_commit(repo, rev, path): # This is based on this comment: # https://github.com/gitpython-developers/GitPython/issues/60#issuecomment-23558741 # And we expect to set max handles to a low value, like 64. - # You should set ulimit -n X, see .travis.yml - # The loops below would easily create 500 handles if these would leak (4 pipes + multiple mapped files). + # You should set ulimit -n X. See .travis.yml. + # The loops below would easily create 500 handles if these would leak + # (4 pipes + multiple mapped files). for _ in range(64): for repo_type in (GitCmdObjectDB, GitDB): repo = Repo(self.rorepo.working_tree_dir, odbt=repo_type) @@ -1200,8 +1202,8 @@ def test_empty_repo(self, rw_dir): self.assertEqual(r.active_branch.name, "master") assert not r.active_branch.is_valid(), "Branch is yet to be born" - # Actually, when trying to create a new branch without a commit, git itself fails. - # We should, however, not fail ungracefully. + # Actually, when trying to create a new branch without a commit, git itself + # fails. We should, however, not fail ungracefully. self.assertRaises(BadName, r.create_head, "foo") self.assertRaises(BadName, r.create_head, "master") # It's expected to not be able to access a tree @@ -1315,13 +1317,13 @@ def test_git_work_tree_dotgit(self, rw_dir): repo = Repo(worktree_path) self.assertIsInstance(repo, Repo) - # This ensures we're able to actually read the refs in the tree, which - # means we can read commondir correctly. + # This ensures we're able to actually read the refs in the tree, which means we + # can read commondir correctly. commit = repo.head.commit self.assertIsInstance(commit, Object) - # This ensures we can read the remotes, which confirms we're reading - # the config correctly. + # This ensures we can read the remotes, which confirms we're reading the config + # correctly. origin = repo.remotes.origin self.assertIsInstance(origin, Remote) diff --git a/test/test_submodule.py b/test/test_submodule.py index 993f6b57e..68164729b 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -94,13 +94,15 @@ def _do_base_tests(self, rwrepo): # The module is not checked-out yet. self.assertRaises(InvalidGitRepositoryError, sm.module) - # ...which is why we can't get the branch either - it points into the module() repository. + # ...which is why we can't get the branch either - it points into the module() + # repository. self.assertRaises(InvalidGitRepositoryError, getattr, sm, "branch") # branch_path works, as it's just a string. assert isinstance(sm.branch_path, str) - # Some commits earlier we still have a submodule, but it's at a different commit. + # Some commits earlier we still have a submodule, but it's at a different + # commit. smold = next(Submodule.iter_items(rwrepo, self.k_subm_changed)) assert smold.binsha != sm.binsha assert smold != sm # the name changed @@ -141,11 +143,12 @@ def _do_base_tests(self, rwrepo): smold.set_parent_commit(self.k_subm_changed + "~1") assert smold.binsha != sm.binsha - # Raises if the sm didn't exist in new parent - it keeps its - # parent_commit unchanged. + # Raises if the sm didn't exist in new parent - it keeps its parent_commit + # unchanged. self.assertRaises(ValueError, smold.set_parent_commit, self.k_no_subm_tag) - # TEST TODO: If a path is in the .gitmodules file, but not in the index, it raises. + # TODO: Test that, if a path is in the .gitmodules file, but not in the index, + # then it raises. # TEST UPDATE ############## @@ -196,8 +199,8 @@ def _do_base_tests(self, rwrepo): # INTERLEAVE ADD TEST ##################### - # url must match the one in the existing repository (if submodule name suggests a new one) - # or we raise. + # url must match the one in the existing repository (if submodule name + # suggests a new one) or we raise. self.assertRaises( ValueError, Submodule.add, @@ -228,7 +231,8 @@ def _do_base_tests(self, rwrepo): assert not csm.module_exists() csm_repopath = csm.path - # Adjust the path of the submodules module to point to the local destination. + # Adjust the path of the submodules module to point to the local + # destination. new_csmclone_path = Git.polish_url(osp.join(self.rorepo.working_tree_dir, sm.path, csm.path)) with csm.config_writer() as writer: writer.set_value("url", new_csmclone_path) @@ -249,7 +253,8 @@ def _do_base_tests(self, rwrepo): # This flushed in a sub-submodule. assert len(list(rwrepo.iter_submodules())) == 2 - # Reset both heads to the previous version, verify that to_latest_revision works. + # Reset both heads to the previous version, verify that to_latest_revision + # works. smods = (sm.module(), csm.module()) for repo in smods: repo.head.reset("HEAD~2", working_tree=1) @@ -296,8 +301,8 @@ def _do_base_tests(self, rwrepo): # Must delete something. self.assertRaises(ValueError, csm.remove, module=False, configuration=False) - # module() is supposed to point to gitdb, which has a child-submodule whose URL is still pointing - # to GitHub. To save time, we will change it to: + # module() is supposed to point to gitdb, which has a child-submodule whose + # URL is still pointing to GitHub. To save time, we will change it to: csm.set_parent_commit(csm.repo.head.commit) with csm.config_writer() as cw: cw.set_value("url", self._small_repo_url()) @@ -399,8 +404,8 @@ def _do_base_tests(self, rwrepo): rwrepo.index.commit("my submod commit") assert len(rwrepo.submodules) == 2 - # Needs update, as the head changed. It thinks it's in the history - # of the repo otherwise. + # Needs update, as the head changed. + # It thinks it's in the history of the repo otherwise. nsm.set_parent_commit(rwrepo.head.commit) osm.set_parent_commit(rwrepo.head.commit) @@ -434,7 +439,8 @@ def _do_base_tests(self, rwrepo): # REMOVE 'EM ALL ################ - # If a submodule's repo has no remotes, it can't be added without an explicit url. + # If a submodule's repo has no remotes, it can't be added without an + # explicit url. osmod = osm.module() osm.remove(module=False) @@ -510,7 +516,8 @@ def test_root_module(self, rwrepo): # TEST UPDATE ############# - # Set up a commit that removes existing, adds new and modifies existing submodules. + # Set up a commit that removes existing, adds new and modifies existing + # submodules. rm = RootModule(rwrepo) assert len(rm.children()) == 1 @@ -534,13 +541,15 @@ def test_root_module(self, rwrepo): sm.update(recursive=False) assert sm.module_exists() with sm.config_writer() as writer: - writer.set_value("path", fp) # Change path to something with prefix AFTER url change. + # Change path to something with prefix AFTER url change. + writer.set_value("path", fp) - # Update doesn't fail, because list_items ignores the wrong path in such situations. + # Update doesn't fail, because list_items ignores the wrong path in such + # situations. rm.update(recursive=False) - # Move it properly - doesn't work as it its path currently points to an indexentry - # which doesn't exist (move it to some path, it doesn't matter here). + # Move it properly - doesn't work as it its path currently points to an + # indexentry which doesn't exist (move it to some path, it doesn't matter here). self.assertRaises(InvalidGitRepositoryError, sm.move, pp) # Reset the path(cache) to where it was, now it works. sm.path = prep @@ -588,23 +597,27 @@ def test_root_module(self, rwrepo): rm.update(recursive=False, dry_run=True, force_remove=True) assert osp.isdir(smp) - # When removing submodules, we may get new commits as nested submodules are auto-committing changes - # to allow deletions without force, as the index would be dirty otherwise. + # When removing submodules, we may get new commits as nested submodules are + # auto-committing changes to allow deletions without force, as the index would + # be dirty otherwise. # QUESTION: Why does this seem to work in test_git_submodule_compatibility() ? self.assertRaises(InvalidGitRepositoryError, rm.update, recursive=False, force_remove=False) rm.update(recursive=False, force_remove=True) assert not osp.isdir(smp) - # 'Apply work' to the nested submodule and ensure this is not removed/altered during updates - # Need to commit first, otherwise submodule.update wouldn't have a reason to change the head. + # 'Apply work' to the nested submodule and ensure this is not removed/altered + # during updates. We need to commit first, otherwise submodule.update wouldn't + # have a reason to change the head. touch(osp.join(nsm.module().working_tree_dir, "new-file")) - # We cannot expect is_dirty to even run as we wouldn't reset a head to the same location. + # We cannot expect is_dirty to even run as we wouldn't reset a head to the same + # location. assert nsm.module().head.commit.hexsha == nsm.hexsha nsm.module().index.add([nsm]) nsm.module().index.commit("added new file") rm.update(recursive=False, dry_run=True, progress=prog) # Would not change head, and thus doesn't fail. - # Everything we can do from now on will trigger the 'future' check, so no is_dirty() check will even run. - # This would only run if our local branch is in the past and we have uncommitted changes. + # Everything we can do from now on will trigger the 'future' check, so no + # is_dirty() check will even run. This would only run if our local branch is in + # the past and we have uncommitted changes. prev_commit = nsm.module().head.commit rm.update(recursive=False, dry_run=False, progress=prog) @@ -616,8 +629,8 @@ def test_root_module(self, rwrepo): # Change url... # ============= - # ...to the first repository. This way we have a fast checkout, and a completely different - # repository at the different url. + # ...to the first repository. This way we have a fast checkout, and a completely + # different repository at the different url. nsm.set_parent_commit(csmremoved) nsmurl = Git.polish_url(osp.join(self.rorepo.working_tree_dir, rsmsp[0])) with nsm.config_writer() as writer: @@ -637,16 +650,15 @@ def test_root_module(self, rwrepo): assert len(rwrepo.submodules) == 1 assert not rwrepo.submodules[0].children()[0].module_exists(), "nested submodule should not be checked out" - # Add the submodule's changed commit to the index, which is what the - # user would do. - # Beforehand, update our instance's binsha with the new one. + # Add the submodule's changed commit to the index, which is what the user would + # do. Beforehand, update our instance's binsha with the new one. nsm.binsha = nsm.module().head.commit.binsha rwrepo.index.add([nsm]) # Change branch. # ============== - # We only have one branch, so we switch to a virtual one, and back - # to the current one to trigger the difference. + # We only have one branch, so we switch to a virtual one, and back to the + # current one to trigger the difference. cur_branch = nsm.branch nsmm = nsm.module() prev_commit = nsmm.head.commit @@ -808,8 +820,8 @@ def test_git_submodules_and_add_sm_with_new_commit(self, rwdir): smm.git.add(Git.polish_url(fp)) smm.git.commit(m="new file added") - # Submodules are retrieved from the current commit's tree, therefore we can't really get a new submodule - # object pointing to the new submodule commit. + # Submodules are retrieved from the current commit's tree, therefore we can't + # really get a new submodule object pointing to the new submodule commit. sm_too = parent.submodules["module_moved"] assert parent.head.commit.tree[sm.path].binsha == sm.binsha assert sm_too.binsha == sm.binsha, "cached submodule should point to the same commit as updated one" @@ -848,8 +860,9 @@ def assert_exists(sm, value=True): # END assert_exists - # As git is backwards compatible itself, it would still recognize what we do here... unless we really - # muss it up. That's the only reason why the test is still here... + # As git is backwards compatible itself, it would still recognize what we do + # here... unless we really muss it up. That's the only reason why the test is + # still here... assert len(parent.git.submodule().splitlines()) == 1 module_repo_path = osp.join(sm.module().working_tree_dir, ".git") @@ -885,7 +898,8 @@ def assert_exists(sm, value=True): assert_exists(csm) # Rename nested submodule. - # This name would move itself one level deeper - needs special handling internally. + # This name would move itself one level deeper - needs special handling + # internally. new_name = csm.name + "/mine" assert csm.rename(new_name).name == new_name assert_exists(csm) @@ -1011,13 +1025,15 @@ def test_branch_renames(self, rw_dir): sm_source_repo.index.add([new_file]) sm.repo.index.commit("added new file") - # Change designated submodule checkout branch to the new upstream feature branch. + # Change designated submodule checkout branch to the new upstream feature + # branch. with sm.config_writer() as smcw: smcw.set_value("branch", sm_fb.name) assert sm.repo.is_dirty(index=True, working_tree=False) sm.repo.index.commit("changed submodule branch to '%s'" % sm_fb) - # Verify submodule update with feature branch that leaves currently checked out branch in it's past. + # Verify submodule update with feature branch that leaves currently checked out + # branch in it's past. sm_mod = sm.module() prev_commit = sm_mod.commit() assert sm_mod.head.ref.name == "master" @@ -1029,7 +1045,8 @@ def test_branch_renames(self, rw_dir): assert sm_mod.head.ref.name == sm_fb.name assert sm_mod.commit() == sm_fb.commit - # Create new branch which is in our past, and thus seemingly unrelated to the currently checked out one. + # Create new branch which is in our past, and thus seemingly unrelated to the + # currently checked out one. # To make it even 'harder', we shall fork and create a new commit. sm_pfb = sm_source_repo.create_head("past-feature", commit="HEAD~20") sm_pfb.checkout() @@ -1043,8 +1060,8 @@ def test_branch_renames(self, rw_dir): # Test submodule updates - must fail if submodule is dirty. touch(osp.join(sm_mod.working_tree_dir, "unstaged file")) - # This doesn't fail as our own submodule binsha didn't change, and the reset is only triggered if - # to_latest_revision is True. + # This doesn't fail as our own submodule binsha didn't change, and the reset is + # only triggered if to_latest_revision is True. parent_repo.submodule_update(to_latest_revision=False) assert sm_mod.head.ref.name == sm_pfb.name, "should have been switched to past head" assert sm_mod.commit() == sm_fb.commit, "Head wasn't reset" @@ -1184,8 +1201,8 @@ def test_submodule_add_unsafe_url_allowed(self, rw_repo): "fd::/foo", ] for url in urls: - # The URL will be allowed into the command, but the command will - # fail since we don't have that protocol enabled in the Git config file. + # The URL will be allowed into the command, but the command will fail + # since we don't have that protocol enabled in the Git config file. with self.assertRaises(GitCommandError): Submodule.add(rw_repo, "new", "new", url, allow_unsafe_protocols=True) assert not tmp_file.exists() @@ -1269,8 +1286,8 @@ def test_submodule_update_unsafe_url_allowed(self, rw_repo): ] for url in urls: submodule = Submodule(rw_repo, b"\0" * 20, name="new", path="new", url=url) - # The URL will be allowed into the command, but the command will - # fail since we don't have that protocol enabled in the Git config file. + # The URL will be allowed into the command, but the command will fail + # since we don't have that protocol enabled in the Git config file. with self.assertRaises(GitCommandError): submodule.update(allow_unsafe_protocols=True) assert not tmp_file.exists() diff --git a/test/test_util.py b/test/test_util.py index 65f77082d..824b3ab3d 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -153,7 +153,8 @@ def _patch_for_wrapping_test(self, mocker, hide_windows_known_errors): reason="PermissionError is only ever wrapped on Windows", ) def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir): - """rmtree wraps PermissionError on Windows when HIDE_WINDOWS_KNOWN_ERRORS is true.""" + """rmtree wraps PermissionError on Windows when HIDE_WINDOWS_KNOWN_ERRORS is + true.""" self._patch_for_wrapping_test(mocker, True) with pytest.raises(SkipTest): @@ -171,7 +172,8 @@ def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir): ], ) def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_tmpdir, hide_windows_known_errors): - """rmtree does not wrap PermissionError on non-Windows systems or when HIDE_WINDOWS_KNOWN_ERRORS is false.""" + """rmtree does not wrap PermissionError on non-Windows systems or when + HIDE_WINDOWS_KNOWN_ERRORS is false.""" self._patch_for_wrapping_test(mocker, hide_windows_known_errors) with pytest.raises(PermissionError): @@ -182,7 +184,9 @@ def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_ @pytest.mark.parametrize("hide_windows_known_errors", [False, True]) def test_does_not_wrap_other_errors(self, tmp_path, mocker, hide_windows_known_errors): - file_not_found_tmpdir = tmp_path / "testdir" # It is deliberately never created. + # The file is deliberately never created. + file_not_found_tmpdir = tmp_path / "testdir" + self._patch_for_wrapping_test(mocker, hide_windows_known_errors) with pytest.raises(FileNotFoundError): @@ -502,7 +506,8 @@ def test_actor_get_uid_laziness_called(self, mock_get_uid): committer = Actor.committer(None) author = Actor.author(None) # We can't test with `self.rorepo.config_reader()` here, as the UUID laziness - # depends on whether the user running the test has their global user.name config set. + # depends on whether the user running the test has their global user.name config + # set. self.assertEqual(committer.name, "user") self.assertTrue(committer.email.startswith("user@")) self.assertEqual(author.name, "user")