Skip to content

gh-137829: Fix shelve tests for backend compatibility #137879

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

gh-137829: Fix shelve tests for backend compatibility #137879

Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
306e43d
Fix shelve tests for backend compatibility
furkanonder Aug 17, 2025
79e335e
split incomplete serializer/deserializer in test_shelve
furkanonder Aug 18, 2025
5b9f61b
add a test case for custom serializer that returning wrong type
furkanonder Aug 18, 2025
2e3ea9b
Add comments clarifying exceptions when serializer returns None
furkanonder Aug 29, 2025
e1b0584
Remove test case
furkanonder Aug 30, 2025
f0af214
Add test for invalid custom serializer
furkanonder Sep 8, 2025
e64777b
Add parameterized test for invalid custom serializers
furkanonder Sep 19, 2025
c683173
Remove test_custom_incomplete_serializer
furkanonder Sep 19, 2025
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
45 changes: 37 additions & 8 deletions Lib/test/test_shelve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ def test_custom_serializer_and_deserializer(self):
def serializer(obj, protocol):
if isinstance(obj, (bytes, bytearray, str)):
if protocol == 5:
if isinstance(obj, bytearray):
return bytes(obj) # DBM backends expect bytes
return obj
return type(obj).__name__
elif isinstance(obj, array.array):
Expand Down Expand Up @@ -222,22 +224,44 @@ def deserializer(data):
s["array_data"], array_data.tobytes().decode()
)

def test_custom_incomplete_serializer_and_deserializer(self):
dbm_sqlite3 = import_helper.import_module("dbm.sqlite3")
def test_custom_incomplete_serializer(self):
os.mkdir(self.dirname)
self.addCleanup(os_helper.rmtree, self.dirname)

with self.assertRaises(dbm_sqlite3.error):
def serializer(obj, protocol=None):
pass
def serializer(obj, protocol=None):
pass

def deserializer(data):
return data.decode("utf-8")
def deserializer(data):
return data.decode("utf-8")

# Since the serializer returns None, dbm.error is raised
# by dbm.sqlite3 and TypeError is raised by other backends.
with self.assertRaises((TypeError, dbm.error)):
with shelve.open(self.fn, serializer=serializer,
deserializer=deserializer) as s:
s["foo"] = "bar"

def test_custom_invalid_serializer(self):
os.mkdir(self.dirname)
self.addCleanup(os_helper.rmtree, self.dirname)

def serializer(obj, protocol=None):
return ["value with invalid type"]
Copy link
Member

@picnixz picnixz Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If you want to test with None and with something that is not None, I would suggest using a parametrized test:

@support.subTests("serialized", [None, ["invalid type"]])
def test_custom_invalid_serializer(self, serialized):
    ...
    def serializer(obj, protocol=None):
        return serialized
    ...

and update the comment saying that the TypeError is due to the return
value of the serializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I've implemented the parameterized test approach as you recommended. I explored two different implementations and wanted to share the trade-offs:

Option 1: Using @subTests decorator (as you suggested)

@subTests("serialized", [None, ["invalid type"]])
def test_custom_invalid_serializer(self, serialized):
    # Create unique directory for each subtest to avoid conflicts
    test_dir = f"{self.dirname}_{id(serialized)}"
    os.mkdir(test_dir)
    self.addCleanup(os_helper.rmtree, test_dir)
    test_fn = os.path.join(test_dir, "shelftemp.db")
    
    def serializer(obj, protocol=None):
        return serialized
    # ... rest of test logic

Option 2: Using self.subTest() with a simple loop

def test_custom_invalid_serializer(self):
    os.mkdir(self.dirname)
    self.addCleanup(os_helper.rmtree, self.dirname)
    
    for serialized in [None, ["invalid type"]]:
        with self.subTest(serialized=serialized):
            def serializer(obj, protocol=None):
                return serialized
            # ... rest of test logic

Trade-offs:

  • Option 1: More explicit parameterization and complete isolation, but requires complex directory management
  • Option 2: Simpler, more readable, and uses standard unittest patterns, but subtests share the same directory

Which approach do you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

Use 1, it was meant for such cases.


def deserializer(data):
return data.decode("utf-8")

# Since the serializer returns None, dbm.error is raised
# by dbm.sqlite3 and TypeError is raised by other backends.
with self.assertRaises((TypeError, dbm.error)):
with shelve.open(self.fn, serializer=serializer,
deserializer=deserializer) as s:
s["foo"] = "bar"

def test_custom_incomplete_deserializer(self):
os.mkdir(self.dirname)
self.addCleanup(os_helper.rmtree, self.dirname)

def serializer(obj, protocol=None):
return type(obj).__name__.encode("utf-8")

Expand Down Expand Up @@ -352,7 +376,7 @@ def type_name_len(obj):
self.assertEqual(s["bytearray_data"], "bytearray")
self.assertEqual(s["array_data"], "array")

def test_custom_incomplete_serializer_and_deserializer_bsd_db_shelf(self):
def test_custom_incomplete_deserializer_bsd_db_shelf(self):
berkeleydb = import_helper.import_module("berkeleydb")
os.mkdir(self.dirname)
self.addCleanup(os_helper.rmtree, self.dirname)
Expand All @@ -370,6 +394,11 @@ def deserializer(data):
self.assertIsNone(s["foo"])
self.assertNotEqual(s["foo"], "bar")

def test_custom_incomplete_serializer_bsd_db_shelf(self):
berkeleydb = import_helper.import_module("berkeleydb")
os.mkdir(self.dirname)
self.addCleanup(os_helper.rmtree, self.dirname)

def serializer(obj, protocol=None):
pass

Expand Down
Loading