Skip to content

Commit

Permalink
Expand "invalid hash" test to assert normal StopIteration
Browse files Browse the repository at this point in the history
Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values (gitpython-developers#1779).

From context, this behavior is clearly not the goal; a bare return
statement should be used here (which has the same effect except for
the `value` attribute of the StopIteration exception). The code had
used a bare return prior to 82b131c (gitpython-developers#1282), when `return` was
changed to `return iter([])`. That was part of a change that added
numerous type annotations. It looks like it was either a mistake,
or possibly an attempt to work around an old bug in a static type
checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.
  • Loading branch information
EliahKagan committed Dec 22, 2023
1 parent 96fc354 commit f5dc1c4
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion test/test_submodule.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,9 @@ def test_iter_items_from_nonexistent_hash(self):
def test_iter_items_from_invalid_hash(self):
"""Check legacy behavaior on BadName (also applies to IOError, i.e. OSError)."""
it = Submodule.iter_items(self.rorepo, "xyz")
with self.assertRaises(StopIteration):
with self.assertRaises(StopIteration) as ctx:
next(it)
self.assertIsNone(ctx.exception.value)

@with_rw_repo(k_no_subm_tag, bare=False)
def test_first_submodule(self, rwrepo):
Expand Down

0 comments on commit f5dc1c4

Please sign in to comment.