From fbc695a018dd3cd49ee24cf6cd84a41642e42270 Mon Sep 17 00:00:00 2001 From: donBarbos Date: Sat, 15 Mar 2025 12:37:36 +0400 Subject: [PATCH 1/6] Add tests for `pickle` command-line interface --- Lib/pickle.py | 8 ++++-- Lib/test/test_pickle.py | 62 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index efcdcbec718166..4fa3632d1a738f 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1907,7 +1907,7 @@ def _loads(s, /, *, fix_imports=True, encoding="ASCII", errors="strict", dump, dumps, load, loads = _dump, _dumps, _load, _loads -if __name__ == "__main__": +def _main(args=None): import argparse import pprint parser = argparse.ArgumentParser( @@ -1915,7 +1915,7 @@ def _loads(s, /, *, fix_imports=True, encoding="ASCII", errors="strict", parser.add_argument( 'pickle_file', nargs='+', help='the pickle file') - args = parser.parse_args() + args = parser.parse_args(args) for fn in args.pickle_file: if fn == '-': obj = load(sys.stdin.buffer) @@ -1923,3 +1923,7 @@ def _loads(s, /, *, fix_imports=True, encoding="ASCII", errors="strict", with open(fn, 'rb') as f: obj = load(f) pprint.pprint(obj) + + +if __name__ == "__main__": + _main() diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 385b257164ef95..34fbdcd48beb56 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -1,18 +1,22 @@ from _compat_pickle import (IMPORT_MAPPING, REVERSE_IMPORT_MAPPING, NAME_MAPPING, REVERSE_NAME_MAPPING) import builtins -import pickle -import io import collections +import contextlib +import io +import pickle import struct import sys +import tempfile import warnings import weakref import doctest import unittest +from io import StringIO +from textwrap import dedent from test import support -from test.support import import_helper +from test.support import import_helper, os_helper from test.pickletester import AbstractHookTests from test.pickletester import AbstractUnpickleTests @@ -699,6 +703,58 @@ def test_multiprocessing_exceptions(self): ('multiprocessing.context', name)) +class CommandLineTest(unittest.TestCase): + def setUp(self): + self.filename = tempfile.mktemp() + self.addCleanup(os_helper.unlink, self.filename) + + @staticmethod + def text_normalize(string): + """Dedent *string* and strip it from its surrounding whitespaces. + + This method is used by the other utility functions so that any + string to write or to match against can be freely indented. + """ + return dedent(string).strip() + + def set_pickle_data(self, data): + with open(self.filename, 'wb') as f: + pickle.dump(data, f) + + def invoke_pickle(self, *flags): + output = StringIO() + with contextlib.redirect_stdout(output): + pickle._main(args=[*flags, self.filename]) + return self.text_normalize(output.getvalue()) + + def test_invocation(self): + # test 'python -m pickle pickle_file' + data = { + 'a': [1, 2.0, 3+4j], + 'b': ('character string', b'byte string'), + 'c': {None, True, False} + } + expect = ''' + {'a': [1, 2.0, (3+4j)], + 'b': ('character string', b'byte string'), + 'c': {None, True, False}} + ''' + self.set_pickle_data(data) + + with self.subTest(data=data): + res = self.invoke_pickle() + expect = self.text_normalize(expect) + self.assertListEqual(res.splitlines(), expect.splitlines()) + + def test_unknown_flag(self): + data = 'some_text' + self.set_pickle_data(data) + with self.assertRaises(SystemExit): + # suppress argparse error message + with contextlib.redirect_stderr(StringIO()): + _ = self.invoke_pickle('--unknown') + + def load_tests(loader, tests, pattern): tests.addTest(doctest.DocTestSuite(pickle)) return tests From 8f720aeab4533dcaa18d9b985ac834b240c5fcef Mon Sep 17 00:00:00 2001 From: donBarbos Date: Sat, 15 Mar 2025 13:03:02 +0400 Subject: [PATCH 2/6] Use string instead of set --- Lib/test/test_pickle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 34fbdcd48beb56..a6462d95948082 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -732,12 +732,12 @@ def test_invocation(self): data = { 'a': [1, 2.0, 3+4j], 'b': ('character string', b'byte string'), - 'c': {None, True, False} + 'c': "string" } expect = ''' {'a': [1, 2.0, (3+4j)], 'b': ('character string', b'byte string'), - 'c': {None, True, False}} + 'c': 'string'} ''' self.set_pickle_data(data) From 2cf119c360df15b80dbfc8b66e03ad0968ac27c4 Mon Sep 17 00:00:00 2001 From: donBarbos Date: Sat, 15 Mar 2025 16:53:09 +0400 Subject: [PATCH 3/6] remove second io import --- Lib/test/test_pickle.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index a6462d95948082..8dcbe882002633 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -13,7 +13,6 @@ import doctest import unittest -from io import StringIO from textwrap import dedent from test import support from test.support import import_helper, os_helper @@ -722,7 +721,7 @@ def set_pickle_data(self, data): pickle.dump(data, f) def invoke_pickle(self, *flags): - output = StringIO() + output = io.StringIO() with contextlib.redirect_stdout(output): pickle._main(args=[*flags, self.filename]) return self.text_normalize(output.getvalue()) @@ -751,7 +750,7 @@ def test_unknown_flag(self): self.set_pickle_data(data) with self.assertRaises(SystemExit): # suppress argparse error message - with contextlib.redirect_stderr(StringIO()): + with contextlib.redirect_stderr(io.StringIO()): _ = self.invoke_pickle('--unknown') From 6ebb9116448a0a440d83b63d700d8a7b48c4ece2 Mon Sep 17 00:00:00 2001 From: donBarbos Date: Sat, 15 Mar 2025 17:24:43 +0400 Subject: [PATCH 4/6] Use '' --- Lib/test/test_pickle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 8dcbe882002633..e978c48c53f33d 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -731,7 +731,7 @@ def test_invocation(self): data = { 'a': [1, 2.0, 3+4j], 'b': ('character string', b'byte string'), - 'c': "string" + 'c': 'string' } expect = ''' {'a': [1, 2.0, (3+4j)], From 864f656e4e7744dd8de50636a514cd0a64ee7efe Mon Sep 17 00:00:00 2001 From: donBarbos Date: Sun, 30 Mar 2025 16:19:09 +0400 Subject: [PATCH 5/6] Add suggestion --- Lib/test/test_pickle.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index e978c48c53f33d..c98430e042f964 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -746,12 +746,15 @@ def test_invocation(self): self.assertListEqual(res.splitlines(), expect.splitlines()) def test_unknown_flag(self): - data = 'some_text' - self.set_pickle_data(data) + output = io.StringIO() with self.assertRaises(SystemExit): # suppress argparse error message - with contextlib.redirect_stderr(io.StringIO()): + with contextlib.redirect_stderr(output): _ = self.invoke_pickle('--unknown') + msg = output.getvalue() + self.assertTrue(msg.startswith('usage: '), + "Output does not start with 'usage: '. " + f"Output was: {msg}") def load_tests(loader, tests, pattern): From 685691810d828cc7e196887207c7e15cd8c62413 Mon Sep 17 00:00:00 2001 From: donBarbos Date: Mon, 31 Mar 2025 13:53:32 +0400 Subject: [PATCH 6/6] Add suggestions --- Lib/test/test_pickle.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index c98430e042f964..296d4b882e138c 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -10,10 +10,10 @@ import tempfile import warnings import weakref +from textwrap import dedent import doctest import unittest -from textwrap import dedent from test import support from test.support import import_helper, os_helper @@ -746,15 +746,12 @@ def test_invocation(self): self.assertListEqual(res.splitlines(), expect.splitlines()) def test_unknown_flag(self): - output = io.StringIO() + stderr = io.StringIO() with self.assertRaises(SystemExit): - # suppress argparse error message - with contextlib.redirect_stderr(output): + # check that the parser help is shown + with contextlib.redirect_stderr(stderr): _ = self.invoke_pickle('--unknown') - msg = output.getvalue() - self.assertTrue(msg.startswith('usage: '), - "Output does not start with 'usage: '. " - f"Output was: {msg}") + self.assertStartsWith(stderr.getvalue(), 'usage: ') def load_tests(loader, tests, pattern):