Skip to content

Better document IterableObj.iter_items and improve some subclasses #1780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 23, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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 (#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 (#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
commit f5dc1c4713dfb937d45d10a595ac879d6e76481c
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