-
-
Notifications
You must be signed in to change notification settings - Fork 33.4k
gh-123309: Add more tests for the pickletools module #123355
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
gh-123309: Add more tests for the pickletools module #123355
Conversation
Add tests for genops() and dis().
|
|
||
| def read(self, n): | ||
| data = self.data[self.pos: self.pos + n] | ||
| self.pos += n |
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.
Probably not applicable, but what if self.pos+n < len(self.data)?
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.
You perhaps mean self.pos+n > len(self.data)? We will get truncated data, and the following reads will return an empty bytes object. Like in real files or BytesIO.
SimpleReader is not currently used in such tests, because it does not make difference, but I am sure that tests for truncated data would pass with it.
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.
Perfect then.
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
…123355) Add tests for genops() and dis(). (cherry picked from commit e5a567b) Co-authored-by: Serhiy Storchaka <[email protected]>
|
GH-123533 is a backport of this pull request to the 3.13 branch. |
…123355) Add tests for genops() and dis(). (cherry picked from commit e5a567b) Co-authored-by: Serhiy Storchaka <[email protected]>
|
GH-123534 is a backport of this pull request to the 3.12 branch. |
… (#123534) Add tests for genops() and dis(). (cherry picked from commit e5a567b) Co-authored-by: Serhiy Storchaka <[email protected]>
Add tests for genops() and dis().