Merge branch 'tickets/DM-25028'

python/lsst/daf/butler/cli/butler.py

@@ -253,7 +253,7 @@ def get_command(self, context, name):
         return doImport(commands[name][0] + "." + cmdNameToFuncName(name))
 
 
-@click.command(cls=LoaderCLI)
+@click.command(cls=LoaderCLI, context_settings=dict(help_option_names=["-h", "--help"]))
 @click.option("--log-level",
               type=click.Choice(["critical", "error", "warning", "info", "debug",
                                  "CRITICAL", "ERROR", "WARNING", "INFO", "DEBUG"]),

python/lsst/daf/butler/cli/cmd/commands.py

@@ -21,8 +21,7 @@
 
 import click
 
-from ..opt import (config_file_option, dataset_type_option, directory_argument, repo_argument, run_option,
-                   transfer_option)
+from ..opt import dataset_type_option, directory_argument, repo_argument, run_option, transfer_option
 from ..utils import split_commas, cli_handle_exception, typeStrAcceptsMultiple
 from ...script import butlerImport, createRepo, configDump, configValidate
 
@@ -50,10 +49,10 @@ def butler_import(*args, **kwargs):
 
 @click.command()
 @repo_argument(help=repo_argument.will_create_repo)
-@config_file_option(help="Path to an existing YAML config file to apply (on top of defaults).")
+@click.option("--seed-config", help="Path to an existing YAML config file to apply (on top of defaults).")
 @click.option("--standalone", is_flag=True, help="Include all defaults in the config file in the repo, "
               "insulating the repo from changes in package defaults.")
-@click.option("--override", "-o", is_flag=True, help="Allow values in the supplied config to override any "
+@click.option("--override", is_flag=True, help="Allow values in the supplied config to override all "
               "repo settings.")
 @click.option("--outfile", "-f", default=None, type=str, help="Name of output file to receive repository "
               "configuration. Default is to write butler.yaml into the specified repo.")

python/lsst/daf/butler/cli/opt/config_file.py

@@ -25,7 +25,8 @@
 
 class config_file_option:  # noqa: N801
 
-    defaultHelp = "The path to the config file."
+    defaultHelp = "Path to a pex config override to be included after the Instrument config overrides are " \
+                  "applied."
 
     def __init__(self, required=False, help=defaultHelp):
         self.required = required

python/lsst/daf/butler/script/createRepo.py

@@ -22,15 +22,15 @@
 from .. import Butler, Config
 
 
-def createRepo(repo, config_file=None, standalone=False, override=False, outfile=None):
+def createRepo(repo, seed_config=None, standalone=False, override=False, outfile=None):
     """Create an empty Gen3 Butler repository.
 
     Parameters
     ----------
     repo : `str`
         URI to the location to create the repo.
-    config_file : `str` or `None`
-        Path to a config yaml file, by default None
+    seed_config : `str` or `None`
+        Path to an existing YAML config file to apply (on top of defaults).
     standalone : `bool`
         Include all the defaults in the config file in the repo if True.
         Insulates the the repo from changes to package defaults. By default
@@ -42,6 +42,6 @@ def createRepo(repo, config_file=None, standalone=False, override=False, outfile
         Name of output file to receive repository configuration. Default is to
         write butler.yaml into the specified repo, by default False.
     """
-    config = Config(config_file) if config_file is not None else None
+    config = Config(seed_config) if seed_config is not None else None
     Butler.makeRepo(repo, config=config, standalone=standalone, forceConfigRoot=not override,
                     outfile=outfile)

python/lsst/daf/butler/tests/mockeredTest.py

@@ -0,0 +1,104 @@
+# This file is part of daf_butler.
+#
+# Developed for the LSST Data Management System.
+# This product includes software developed by the LSST Project
+# (http://www.lsst.org).
+# See the COPYRIGHT file at the top-level directory of this distribution
+# for details of code ownership.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import abc
+import click
+import click.testing
+import copy
+import unittest
+
+from ..cli.utils import clickResultMsg, mockEnvVar, Mocker
+from ..cli import butler
+
+
+class MockeredTestBase(unittest.TestCase, abc.ABC):
+    """A test case base that is used to verify click command functions import
+    and call their respective script fucntions correctly.
+    """
+
+    @classmethod
+    @property
+    @abc.abstractmethod
+    def defaultExpected(cls):
+        pass
+
+    def setUp(self):
+        self.runner = click.testing.CliRunner(env=mockEnvVar)
+
+    def makeExpected(self, **kwargs):
+        expected = copy.copy(self.defaultExpected)
+        expected.update(kwargs)
+        return expected
+
+    def run_command(self, inputs):
+        """Use the CliRunner with the mock environment variable set to execute
+        a butler subcommand and parameters specified in inputs.
+
+        Parameters
+        ----------
+        inputs : [`str`]
+            A list of strings that begins with the subcommand name and is
+            followed by arguments, option keys and option values.
+
+        Returns
+        -------
+        result : `click.testing.Result`
+            The Result object contains the results from calling
+            self.runner.invoke.
+        """
+        return self.runner.invoke(butler.cli, inputs)
+
+    def run_test(self, inputs, expectedKwargs):
+        """Run the subcommand specified in inputs and verify a successful
+        outcome where exit code = 0 and the mock object has been called with
+        the expected arguments.
+
+        Parameters
+        ----------
+        inputs : [`str`]
+            A list of strings that begins with the subcommand name and is
+            followed by arguments, option keys and option values.
+        expectedKwargs : `dict` [`str`, `str`]
+            The arguments that the subcommand function is expected to have been
+            called with. Keys are the argument name and values are the argument
+            value.
+        """
+        result = self.run_command(inputs)
+        self.assertEqual(result.exit_code, 0, clickResultMsg(result))
+        Mocker.mock.assert_called_with(**expectedKwargs)
+
+    def run_missing(self, inputs, expectedMsg):
+        """Run the subcommand specified in inputs and verify a failed outcome
+        where exit code != 0 and an expected message has been written to
+        stdout.
+
+        Parameters
+        ----------
+        inputs : [`str`]
+            A list of strings that begins with the subcommand name and is
+            followed by arguments, option keys and option values.
+        expectedMsg : `str`
+            An error message that should be present in stdout after running the
+            subcommand.
+        """
+        result = self.run_command(inputs)
+        self.assertNotEqual(result.exit_code, 0, clickResultMsg(result))
+        self.assertIn(expectedMsg, result.stdout)

tests/test_cliCmdImport.py

@@ -22,83 +22,67 @@
 """Unit tests for daf_butler CLI config-dump command.
 """
 
-import click
-import click.testing
 import unittest
+import unittest.mock
 
-from lsst.daf.butler.cli import butler
-from lsst.daf.butler.cli.utils import clickResultMsg, Mocker, mockEnvVar
+from lsst.daf.butler.tests.mockeredTest import MockeredTestBase
+from lsst.daf.butler.cli.utils import Mocker
 
 
-def makeExpectedKwargs(**kwargs):
-    expected = dict(repo=None,
-                    transfer="auto",
-                    output_run=None,
-                    directory=None,
-                    export_file=None)
-    expected.update(kwargs)
-    return expected
+class ImportTestCase(MockeredTestBase):
 
-
-class Case(unittest.TestCase):
-
-    def setUp(self):
-        self.runner = click.testing.CliRunner(env=mockEnvVar)
-
-    def run_test(self, inputs, expectedKwargs):
-        """Test command line interaction with import command function.
-
-        Parameters
-        ----------
-        inputs : [`str`]
-            A list of the arguments to the butler command, starting with
-            `import`
-        expectedKwargs : `dict` [`str`, `str`]
-            The expected arguments to the import command function, keys are
-            the argument name and values are the argument value.
-        """
-        result = self.runner.invoke(butler.cli, inputs)
-        self.assertEqual(result.exit_code, 0, clickResultMsg(result))
-        Mocker.mock.assert_called_with(**expectedKwargs)
+    defaultExpected = dict(repo=None,
+                           transfer="auto",
+                           output_run=None,
+                           directory=None,
+                           export_file=None)
 
     def test_minimal(self):
         """Test only the required parameters, and omit the optional parameters.
         """
-        expected = makeExpectedKwargs(repo="here", directory="foo", output_run="out")
-        self.run_test(["import", "here",
-                       "foo",
-                       "--output-run", "out"], expected)
+        self.run_test(["import", "here", "foo",
+                       "--output-run", "out"],
+                      self.makeExpected(repo="here", directory="foo",
+                                        output_run="out"))
 
     def test_almostAll(self):
         """Test all the parameters, except export_file which gets its own test
         case below.
         """
-        with self.runner.isolated_filesystem():
-            expected = makeExpectedKwargs(repo="here", directory="foo", output_run="out", transfer="symlink")
-            self.run_test(["import", "here",
-                           "foo",
-                           "--output-run", "out",
-                           "--transfer", "symlink"], expected)
+        self.run_test(["import", "here", "foo",
+                       "--output-run", "out",
+                       "--transfer", "symlink"],
+                      self.makeExpected(repo="here", directory="foo",
+                                        output_run="out",
+                                        transfer="symlink"))
 
     def test_missingArgument(self):
-        """Verify the command fails if a positional argument is missing"""
-        runner = click.testing.CliRunner(env=mockEnvVar)
-        result = runner.invoke(butler.cli, ["import", "foo", "--output-run", "out"])
-        self.assertNotEqual(result.exit_code, 0, clickResultMsg(result))
+        """Verify the command fails if either of the positional arguments,
+        REPO or DIRECTORY, is missing."""
+        self.run_missing(["import", "foo", "--output-run", "out"],
+                         'Error: Missing argument "DIRECTORY".')
 
 
-class ExportFileCase(unittest.TestCase):
+class ExportFileCase(MockeredTestBase):
 
     didRead = None
 
+    defaultExpected = dict(repo=None,
+                           transfer="auto",
+                           output_run=None,
+                           directory=None,
+                           export_file=None)
+
     def setUp(self):
         # add a side effect to Mocker so that it will call our method when it
         # is called.
         Mocker.mock.side_effect = self.read_test
+        super().setUp()
 
     def tearDown(self):
         # reset the Mocker's side effect on our way out!
         Mocker.mock.side_effect = None
+        super().tearDown()
 
     @staticmethod
     def read_test(*args, **kwargs):
@@ -113,14 +97,22 @@ def read_test(*args, **kwargs):
     def test_exportFile(self):
         """Test all the parameters, except export_file.
         """
-        runner = click.testing.CliRunner(env=mockEnvVar)
-        with runner.isolated_filesystem():
+        # export_file is ANY in makeExpected because that variable is opened by
+        # click and the open handle is passed to the command function as a
+        # TestIOWrapper. It doesn't work to test it with
+        # MagicMock.assert_called_with because if a TextIOWrapper is created
+        # here it will be a different instance and not compare equal. We test
+        # that variable via the mocker.side_effect used in self.read_test.
+        with self.runner.isolated_filesystem():
             f = open("output.yaml", "w")
             f.write("foobarbaz")
             f.close()
-            result = runner.invoke(butler.cli, ["import", "here", "foo", "--output-run", "out",
-                                                "--export-file", "output.yaml"])
-            self.assertEqual(result.exit_code, 0, clickResultMsg(result))
+            self.run_test(["import", "here", "foo",
+                           "--output-run", "out",
+                           "--export-file", "output.yaml"],
+                          self.makeExpected(repo="here", directory="foo",
+                                            output_run="out",
+                                            export_file=unittest.mock.ANY))
             self.assertEqual("foobarbaz", ExportFileCase.didRead)
 
 

tests/test_cliUtils.py

@@ -39,7 +39,7 @@ def test_callMock(self):
         runner = click.testing.CliRunner(env=mockEnvVar)
         result = runner.invoke(butler.cli, ["create", "repo"])
         self.assertEqual(result.exit_code, 0, f"output: {result.output} exception: {result.exception}")
-        Mocker.mock.assert_called_with(repo="repo", config_file=None, standalone=False, override=False,
+        Mocker.mock.assert_called_with(repo="repo", seed_config=None, standalone=False, override=False,
                                        outfile=None)