From 62bf3b1ea0a820e07a15859ddaf759fa11f384a2 Mon Sep 17 00:00:00 2001 From: Liz Gehret Date: Wed, 10 Jan 2024 15:45:18 -0700 Subject: [PATCH 1/2] BUG: missing metadata column error not raised correctly --- q2cli/click/option.py | 13 +++++++++++-- q2cli/tests/test_cli.py | 12 +++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/q2cli/click/option.py b/q2cli/click/option.py index f2567823..3c4b41eb 100644 --- a/q2cli/click/option.py +++ b/q2cli/click/option.py @@ -107,13 +107,22 @@ def consume_value(self, ctx, opts): def _consume_metadata(self, ctx, opts): # double consume + # this consume deals with the metadata file md_file, source = super().consume_value(ctx, opts) # consume uses self.name, so mutate but backup for after backup, self.name = self.name, self.q2_extra_dest - md_col, _ = super().consume_value(ctx, opts) + try: + # this consume deals with the metadata column + md_col, _ = super().consume_value(ctx, opts) + # If `--m-metadata-column` isn't provided, need to set md_col to None + # in order for the click.MissingParameter errors below to be raised + except click.MissingParameter: + md_col = None self.name = backup - + # These branches won't get hit unless there's a value associated with + # md_col - the try/except case above handled the situation where the + # metadata_column parameter itself wasn't provided (vs just a value) if (md_col is None) != (md_file is None): # missing one or the other if md_file is None: diff --git a/q2cli/tests/test_cli.py b/q2cli/tests/test_cli.py index de4689a0..2495cf0c 100644 --- a/q2cli/tests/test_cli.py +++ b/q2cli/tests/test_cli.py @@ -569,7 +569,7 @@ def test_invalid_metadata_merge(self): class TestMetadataColumnSupport(MetadataTestsBase): - def test_required_missing(self): + def test_required_missing_file(self): result = self._run_command( 'identity-with-metadata-column', '--i-ints', self.input_artifact, '--o-out', self.output_artifact) @@ -578,6 +578,16 @@ def test_required_missing(self): self.assertTrue(result.output.startswith('Usage:')) self.assertIn("Missing option '--m-metadata-file'", result.output) + def test_required_missing_column(self): + result = self._run_command( + 'identity-with-metadata-column', '--i-ints', self.input_artifact, + '--m-metadata-file', self.metadata_file1, + '--o-out', self.output_artifact) + + self.assertEqual(result.exit_code, 1) + self.assertTrue(result.output.startswith('Usage:')) + self.assertIn("Missing option '--m-metadata-column'", result.output) + def test_optional_metadata_missing(self): result = self._run_command( 'identity-with-optional-metadata-column', '--i-ints', From 23785f8f720d90567fbfd5cbf0890fccd43005dd Mon Sep 17 00:00:00 2001 From: Liz Gehret Date: Wed, 17 Jan 2024 12:14:38 -0700 Subject: [PATCH 2/2] adding test case for missing md col and file --- q2cli/tests/test_cli.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/q2cli/tests/test_cli.py b/q2cli/tests/test_cli.py index 2495cf0c..76a3c6d9 100644 --- a/q2cli/tests/test_cli.py +++ b/q2cli/tests/test_cli.py @@ -569,7 +569,8 @@ def test_invalid_metadata_merge(self): class TestMetadataColumnSupport(MetadataTestsBase): - def test_required_missing_file(self): + # Neither md file or column params provided + def test_required_missing_file_and_column(self): result = self._run_command( 'identity-with-metadata-column', '--i-ints', self.input_artifact, '--o-out', self.output_artifact) @@ -578,6 +579,17 @@ def test_required_missing_file(self): self.assertTrue(result.output.startswith('Usage:')) self.assertIn("Missing option '--m-metadata-file'", result.output) + # md file param missing, md column param & value provided + def test_required_missing_file(self): + result = self._run_command( + 'identity-with-metadata-column', '--i-ints', self.input_artifact, + '--m-metadata-column', 'a', '--o-out', self.output_artifact) + + self.assertEqual(result.exit_code, 1) + self.assertTrue(result.output.startswith('Usage:')) + self.assertIn("Missing option '--m-metadata-file'", result.output) + + # md file param & value provided, md column param missing def test_required_missing_column(self): result = self._run_command( 'identity-with-metadata-column', '--i-ints', self.input_artifact,