Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 9 additions & 0 deletions Lib/os.py
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,15 @@ def __repr__(self):
)
return f"environ({{{formatted_items}}})"

def clear(self):
while self._data:
for key in list(self._data):
unsetenv(key)
try:
del self._data[key]
except KeyError:
pass

def copy(self):
return dict(self)

Expand Down
65 changes: 65 additions & 0 deletions Lib/test/test_os/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,71 @@ def test_reload_environ(self):
self.assertNotIn(b'test_env', os.environb)
self.assertNotIn('test_env', os.environ)

def test_clear_empties_environ(self):
os.environ["test_key_to_clear1"] = "test_value_to_clear1"
os.environ["test_key_to_clear2"] = "test_value_to_clear2"
os.environ["test_key_to_clear3"] = "test_value_to_clear3"
Comment on lines +1498 to +1500
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that 3 variables are useful. IMO a single variable to make sure that os.environ is non-empty is enough.

Suggested change
os.environ["test_key_to_clear1"] = "test_value_to_clear1"
os.environ["test_key_to_clear2"] = "test_value_to_clear2"
os.environ["test_key_to_clear3"] = "test_value_to_clear3"
os.environ["test_key_to_clear"] = "test_value_to_clear"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to also test the edge case when an errorneous os.environ.clear() removes only 1 variable like os.environ.popitem() would do.


# Test environ.clear() removes the environment variables
os.environ.clear()
self.assertEqual(os.environ, {})
if os.supports_bytes_environ:
self.assertEqual(os.environb, {})

# Repeated calls should be idempotent
os.environ.clear()
self.assertEqual(os.environ, {})
if os.supports_bytes_environ:
self.assertEqual(os.environb, {})

@unittest.skipUnless(os.supports_bytes_environ, "os.environb required for this test.")
def test_clear_empties_environb(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is more or less the same as test_clear_empties_environ() but on os.environb. I'm not sure that it's useful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not useful? Otherwise side-effects of environb.clear() are untested.

os.environb[b"test_key_to_clear1"] = b"test_value_to_clear1"
os.environb[b"test_key_to_clear2"] = b"test_value_to_clear2"
os.environb[b"test_key_to_clear3"] = b"test_value_to_clear3"

# Test environ.clear() removes the environment variables
os.environb.clear()
self.assertEqual(os.environ, {})
self.assertEqual(os.environb, {})

# Repeated calls should be idempotent
os.environb.clear()
self.assertEqual(os.environ, {})
self.assertEqual(os.environb, {})

def test_clear_empties_process_environment(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced that this test is useful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? The point of calling unsetenv() is to modify the process-level environment variables.

This test verifies that this modification actually happens in case of environ.clear(). Otherwise we could just do environ._data = {}, and the unit tests would be happy.

# Determine if on the current platform os.unsetenv()
# updates process environment.
os.environ['to_remove'] = 'value'
os.unsetenv('to_remove')
os.reload_environ()
if 'to_remove' in os.environ:
self.skipTest("os.unsetenv() doesn't update the process environment on this platform.")

# Set up two environment variables to be cleared
os.environ["test_env1"] = "some_value1"
os.environ["test_env2"] = "some_value2"

# Ensure the variables were persisted on process level.
os.reload_environ()
self.assertEqual(os.getenv("test_env1"), "some_value1")
self.assertEqual(os.getenv("test_env2"), "some_value2")

# Test that os.clear() clears both os.environ and os.environb
os.environ.clear()
self.assertEqual(os.environ, {})
if os.supports_bytes_environ:
self.assertEqual(os.environb, {})

# Test that os.clear() also clears the process environment,
# so that after os.reload_environ() environ and environb are still empty.
os.reload_environ()
self.assertEqual(os.environ, {})
if os.supports_bytes_environ:
self.assertEqual(os.environb, {})


class WalkTests(unittest.TestCase):
"""Tests for os.walk()."""
is_fwalk = False
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Ensure that :data:`os.environ.clear() <os.environ>`
has linear complexity instead of quadratic complexity.
Loading