From b9aa1ff869f159f3106751b77395c8438b058e5e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 22 Dec 2023 02:16:28 -0500 Subject: [PATCH] Expand "invalid hash" test to assert normal StopIteration 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. 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 (#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. --- test/test_submodule.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 5226d7a6e..993f6b57e 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -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):