From a65c268a91acd8e6333b6818f353f1dd7ae25d07 Mon Sep 17 00:00:00 2001 From: Chia-Hui Lin Date: Fri, 23 Apr 2021 07:30:41 -0700 Subject: [PATCH 1/4] check the args before handle the store function --- synapseclient/__main__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapseclient/__main__.py b/synapseclient/__main__.py index 296b366f7..490509ea5 100644 --- a/synapseclient/__main__.py +++ b/synapseclient/__main__.py @@ -134,7 +134,8 @@ def store(args, syn): # If both args.FILE and args.file specified raise error if args.file and args.FILE: raise ValueError('only specify one file') - + if not args.file and not args.FILE: + raise ValueError(f'For the {args.subparser} command, put the positional FILE as the first arg') _descriptionFile_arg_check(args) args.file = args.FILE if args.FILE is not None else args.file @@ -561,7 +562,7 @@ def build_parser(): parser.add_argument('-s', '--skip-checks', dest='skip_checks', action='store_true', help='suppress checking for version upgrade messages and endpoint redirection') - subparsers = parser.add_subparsers(title='commands', + subparsers = parser.add_subparsers(title='commands', dest='subparser', description='The following commands are available:', help='For additional help: "synapse -h"') From 49072fb8ad7a907539627138941c180f99905cac Mon Sep 17 00:00:00 2001 From: Chia-Hui Lin Date: Fri, 23 Apr 2021 07:40:38 -0700 Subject: [PATCH 2/4] add unit test --- tests/unit/synapseclient/unit_test_commandline.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/synapseclient/unit_test_commandline.py b/tests/unit/synapseclient/unit_test_commandline.py index 79c6c96bd..823cf8974 100644 --- a/tests/unit/synapseclient/unit_test_commandline.py +++ b/tests/unit/synapseclient/unit_test_commandline.py @@ -572,3 +572,16 @@ def test_get__with_normal_id(self, mock_os): call(mock_entity), call('Downloaded file: %s', './base_tmp_path'), call('Creating %s', './tmp_path')] + + +class TestStoreFunction: + @patch('synapseclient.client.Synapse') + def setup(self, mock_syn): + self.syn = mock_syn + + def test_get__without_file_args(self): + parser = cmdline.build_parser() + args = parser.parse_args(['store', '--parentid', 'syn123', '--used', 'syn456']) + with pytest.raises(ValueError) as ve: + cmdline.store(args, self.syn) + assert str(ve.value) == "For the store command, put the positional FILE as the first arg" From 1d3151a267c9da8e47e1e9865dab7da82c04c50c Mon Sep 17 00:00:00 2001 From: Chia-Hui Lin Date: Fri, 23 Apr 2021 07:53:31 -0700 Subject: [PATCH 3/4] fix the integration test issue --- synapseclient/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapseclient/__main__.py b/synapseclient/__main__.py index 490509ea5..a5acde9d0 100644 --- a/synapseclient/__main__.py +++ b/synapseclient/__main__.py @@ -134,7 +134,7 @@ def store(args, syn): # If both args.FILE and args.file specified raise error if args.file and args.FILE: raise ValueError('only specify one file') - if not args.file and not args.FILE: + if args.type == 'File' and not args.file and not args.FILE: raise ValueError(f'For the {args.subparser} command, put the positional FILE as the first arg') _descriptionFile_arg_check(args) From 922106ca3e272f7f37b2a97cf7b40549bfa16c2e Mon Sep 17 00:00:00 2001 From: Chia-Hui Lin Date: Thu, 29 Apr 2021 13:23:56 -0700 Subject: [PATCH 4/4] change the raise value error msg --- synapseclient/__main__.py | 2 +- tests/unit/synapseclient/unit_test_commandline.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapseclient/__main__.py b/synapseclient/__main__.py index a5acde9d0..7971e4e89 100644 --- a/synapseclient/__main__.py +++ b/synapseclient/__main__.py @@ -135,7 +135,7 @@ def store(args, syn): if args.file and args.FILE: raise ValueError('only specify one file') if args.type == 'File' and not args.file and not args.FILE: - raise ValueError(f'For the {args.subparser} command, put the positional FILE as the first arg') + raise ValueError(f'{args.subparser} missing required FILE argument') _descriptionFile_arg_check(args) args.file = args.FILE if args.FILE is not None else args.file diff --git a/tests/unit/synapseclient/unit_test_commandline.py b/tests/unit/synapseclient/unit_test_commandline.py index 823cf8974..3c2ad168e 100644 --- a/tests/unit/synapseclient/unit_test_commandline.py +++ b/tests/unit/synapseclient/unit_test_commandline.py @@ -584,4 +584,4 @@ def test_get__without_file_args(self): args = parser.parse_args(['store', '--parentid', 'syn123', '--used', 'syn456']) with pytest.raises(ValueError) as ve: cmdline.store(args, self.syn) - assert str(ve.value) == "For the store command, put the positional FILE as the first arg" + assert str(ve.value) == "store missing required FILE argument"