Skip to content

Commit e6c11de

Browse files
committed
Address review feedback from serhiy-storchaka
- Use nonlocal close_source instead of _closed flag - Bind warnings.warn as default parameter in __del__ - Use assertWarns(ResourceWarning) instead of adding close() calls - Use support.gc_collect() for consistent test behavior - Remove broad exception handling Signed-off-by: Osama Abdelkader <[email protected]>
1 parent bf3f888 commit e6c11de

File tree

2 files changed

+23
-35
lines changed

2 files changed

+23
-35
lines changed

Lib/test/test_xml_etree.py

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -696,14 +696,15 @@ def test_iterparse(self):
696696
next(it)
697697
self.assertEqual(str(cm.exception),
698698
'junk after document element: line 1, column 12')
699-
it.close() # Close to avoid ResourceWarning
700-
del cm, it
699+
with self.assertWarns(ResourceWarning):
700+
del cm, it
701+
support.gc_collect()
701702

702703
# Deleting iterator without close() should emit ResourceWarning (bpo-43292)
703-
it = iterparse(SIMPLE_XMLFILE)
704-
del it
705-
import gc
706-
gc.collect() # Ensure previous iterator is cleaned up
704+
with self.assertWarns(ResourceWarning):
705+
it = iterparse(SIMPLE_XMLFILE)
706+
del it
707+
support.gc_collect()
707708

708709
# Explicitly calling close() should not emit warning
709710
with warnings_helper.check_no_resource_warning(self):
@@ -712,11 +713,12 @@ def test_iterparse(self):
712713
del it
713714

714715
# Not closing before del should emit ResourceWarning
715-
it = iterparse(SIMPLE_XMLFILE)
716-
action, elem = next(it)
717-
self.assertEqual((action, elem.tag), ('end', 'element'))
718-
del it, elem
719-
gc.collect() # Ensure previous iterator is cleaned up
716+
with self.assertWarns(ResourceWarning):
717+
it = iterparse(SIMPLE_XMLFILE)
718+
action, elem = next(it)
719+
self.assertEqual((action, elem.tag), ('end', 'element'))
720+
del it, elem
721+
support.gc_collect()
720722

721723
with warnings_helper.check_no_resource_warning(self):
722724
it = iterparse(SIMPLE_XMLFILE)
@@ -743,7 +745,7 @@ def create_unclosed():
743745
# Don't close - should warn
744746

745747
create_unclosed()
746-
gc.collect()
748+
support.gc_collect()
747749

748750
resource_warnings = [x for x in w
749751
if issubclass(x.category, ResourceWarning)]
@@ -760,7 +762,7 @@ def create_closed():
760762
context.close()
761763

762764
create_closed()
763-
gc.collect()
765+
support.gc_collect()
764766

765767
resource_warnings = [x for x in w
766768
if issubclass(x.category, ResourceWarning)]
@@ -778,7 +780,7 @@ def create_with_fileobj():
778780
# Don't close - file object managed externally
779781

780782
create_with_fileobj()
781-
gc.collect()
783+
support.gc_collect()
782784

783785
resource_warnings = [x for x in w
784786
if issubclass(x.category, ResourceWarning)]

Lib/xml/etree/ElementTree.py

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,35 +1261,21 @@ def iterator(source):
12611261
gen = iterator(source)
12621262
class IterParseIterator(collections.abc.Iterator):
12631263
__next__ = gen.__next__
1264-
1264+
12651265
def close(self):
1266+
nonlocal close_source
12661267
if close_source:
12671268
source.close()
12681269
gen.close()
1269-
self._closed = True
1270+
close_source = False
12701271

1271-
def __del__(self):
1272-
if close_source and not getattr(self, '_closed', False):
1273-
try:
1274-
warnings.warn(
1275-
f"unclosed file {source!r}",
1276-
ResourceWarning,
1277-
stacklevel=2,
1278-
source=self
1279-
)
1280-
except:
1281-
# Ignore errors during warning emission in __del__
1282-
# This can happen during interpreter shutdown
1283-
pass
1272+
def __del__(self, _warn=warnings.warn):
12841273
if close_source:
1285-
try:
1286-
source.close()
1287-
except:
1288-
# Ignore errors when closing during __del__
1289-
pass
1274+
_warn(f"unclosed file {source!r}",
1275+
ResourceWarning, source=self)
1276+
source.close()
12901277

12911278
it = IterParseIterator()
1292-
it._closed = False
12931279
it.root = None
12941280
wr = weakref.ref(it)
12951281
return it

0 commit comments

Comments
 (0)