-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
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.
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
base: main
Are you sure you want to change the base?
Conversation
I also added a new test case for wrong types from custom serializers. For example when serializers return |
Lib/test/test_shelve.py
Outdated
self.addCleanup(os_helper.rmtree, self.dirname) | ||
|
||
def serializer(obj, protocol=None): | ||
return ["value with invalid type"] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.