Repository: mtreinish/stestr (language: python)

Issue:
pyproject.toml support
Everybody loves `pyproject.toml` for storing all of their project dependencies, metadata, and test tooling configs.  `stestr` should support it.

Ideally, it would simply be a `[tool.stestr]` section, similar to how `tox` support works (but TOML instead of `.ini`, obviously)


Discussion / hints:

Fix patch:
diff --git a/doc/source/MANUAL.rst b/doc/source/MANUAL.rst
index e9decfc..fae7c1a 100644
--- a/doc/source/MANUAL.rst
+++ b/doc/source/MANUAL.rst
@@ -84,24 +84,58 @@ CLI. This way you can run stestr directly without having to write a config file
 and manually specify the test_path like above with the ``--test-path``/``-t``
 CLI argument.
 
+.. _tox:
+
 Tox
 '''
 
 If you are also using `tox <https://tox.readthedocs.io/en/latest/>`__ with your
 project then it is not necessary to create separate stestr config file, instead
 you can embed the necessary configuration in the existing ``tox.ini`` file with
-an ``stestr`` section. For example a full configuration section would be::
+an ``[stestr]`` section. For example a full configuration section would be::
 
   [stestr]
   test_path=./project/tests
   top_dir=./
   group_regex=([^\.]*\.)*
 
+Any configuration directives outside the ``[stestr]`` section will be ignored.
 It's important to note that if either the ``--config``/``-c`` CLI argument is
-specified and pointing to an existing file or the default location
-``.stestr.conf`` file is present then any configuration in the ``tox.ini`` will
-be ignored. Configuration embedded in a ``tox.ini`` will only be used if other
-configuration files are not present.
+specified, or the default location ``.stestr.conf`` file is present
+then any configuration in the ``tox.ini`` will be ignored. Configuration
+embedded in a ``tox.ini`` will only be used if other configuration
+files are not present.
+
+pyproject.toml
+''''''''''''''
+
+Similarly, if your project is using ``pyproject.toml``, you may forego the
+config file, and instead create a ``[tool.stestr]`` section with the desired
+configuration options.  For example::
+
+  [tool.stestr]
+  test_path = "./project/tests"
+  top_dir = "./"
+  group_regex = "([^\.]*\.)*"
+
+The same caveats apply as the :ref:`tox` with regards to CLI arguments.
+
+Configuration file precedence
+'''''''''''''''''''''''''''''
+
+The order in which configuration files are read is as follows:
+
+* Any file specified with the ``--config``/``-c`` CLI argument
+* The ``.stestr.conf`` file
+* The ``[tool.stestr]`` section in a ``pyproject.toml`` file
+* The ``[stestr]`` section in a ``tox.ini`` file
+
+Also of note is that files specified with ``--config-file``/``-c``
+may be either ``.ini`` or TOML format. If providing configs in
+``.ini`` format, they **must** be in a ``[DEFAULT]`` section. If
+providing configs in TOML format, the configuration directives
+**must** be located in a ``[tool.stestr]`` section, and the filename
+**must** have a ``.toml`` extension.
 
 Running tests
 -------------
diff --git a/requirements.txt b/requirements.txt
index 55149fe..91bb1f3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -8,3 +8,4 @@ fixtures>=3.0.0 # Apache-2.0/BSD
 testtools>=2.2.0 # MIT
 PyYAML>=3.10.0 # MIT
 voluptuous>=0.8.9 # BSD License
+tomlkit>=0.11.6 # MIT


Test patch:
diff --git a/stestr/commands/list.py b/stestr/commands/list.py
index 8697ebe..39a32cb 100644
--- a/stestr/commands/list.py
+++ b/stestr/commands/list.py
@@ -13,7 +13,6 @@
 """List the tests from a project and show them."""
 
 from io import BytesIO
-import os
 import sys
 
 from cliff import command
@@ -90,7 +89,7 @@ def take_action(self, parsed_args):
 
 
 def list_command(
-    config=".stestr.conf",
+    config=config_file.TestrConf.DEFAULT_CONFIG_FILENAME,
     repo_url=None,
     test_path=None,
     top_dir=None,
@@ -133,12 +132,7 @@ def list_command(
 
     """
     ids = None
-    if config and os.path.isfile(config):
-        conf = config_file.TestrConf(config)
-    elif os.path.isfile("tox.ini"):
-        conf = config_file.TestrConf("tox.ini", section="stestr")
-    else:
-        conf = config_file.TestrConf(config)
+    conf = config_file.TestrConf.load_from_file(config)
     cmd = conf.get_run_command(
         regexes=filters,
         repo_url=repo_url,
diff --git a/stestr/commands/run.py b/stestr/commands/run.py
index 4cfac09..4afb8b5 100644
--- a/stestr/commands/run.py
+++ b/stestr/commands/run.py
@@ -368,7 +368,7 @@ def gather_errors(test_dict):
 
 
 def run_command(
-    config=".stestr.conf",
+    config=config_file.TestrConf.DEFAULT_CONFIG_FILENAME,
     repo_url=None,
     test_path=None,
     top_dir=None,
@@ -644,12 +644,7 @@ def run_tests():
             # that are both failing and listed.
             ids = list_ids.intersection(ids)
 
-    if config and os.path.isfile(config):
-        conf = config_file.TestrConf(config)
-    elif os.path.isfile("tox.ini"):
-        conf = config_file.TestrConf("tox.ini", section="stestr")
-    else:
-        conf = config_file.TestrConf(config)
+    conf = config_file.TestrConf.load_from_file(config)
     if not analyze_isolation:
         cmd = conf.get_run_command(
             ids,
diff --git a/stestr/config_file.py b/stestr/config_file.py
index 2587b6a..6a66503 100644
--- a/stestr/config_file.py
+++ b/stestr/config_file.py
@@ -15,6 +15,7 @@
 import sys
 
 import configparser
+import tomlkit
 
 from stestr.repository import util
 from stestr import test_processor
@@ -27,17 +28,66 @@ class TestrConf:
     of a tox.ini file the stestr section in the tox.ini file
 
     :param str config_file: The path to the config file to use
-    :param str section: The section to use for the stestr config. By default
-        this is DEFATULT.
+    :param str section: The section to use for the stestr config. By default,
+        this is DEFAULT.
     """
 
+    DEFAULT_CONFIG_FILENAME = ".stestr.conf"
     _escape_trailing_backslash_re = re.compile(r"(?<=[^\\])\\$")
+    # Set sensible config defaults here, so that override methods are kept DRY
+    test_path = None
+    top_dir = None
+    parallel_class = False
+    group_regex = None
 
     def __init__(self, config_file, section="DEFAULT"):
-        self.parser = configparser.ConfigParser()
-        self.parser.read(config_file)
-        self.config_file = config_file
+        self.config_file = str(config_file)
         self.section = section
+        if self.config_file.lower().endswith(".toml"):
+            self._load_from_toml()
+        else:
+            self._load_from_configparser()
+
+    def _load_from_configparser(self):
+        parser = configparser.ConfigParser()
+        parser.read(self.config_file)
+        self.test_path = parser.get(self.section, "test_path", fallback=self.test_path)
+        self.top_dir = parser.get(self.section, "top_dir", fallback=self.top_dir)
+        self.parallel_class = parser.getboolean(
+            self.section, "parallel_class", fallback=self.parallel_class
+        )
+        self.group_regex = parser.get(
+            self.section, "group_regex", fallback=self.group_regex
+        )
+
+    def _load_from_toml(self):
+        with open(self.config_file) as f:
+            doc = tomlkit.load(f)
+            root = doc["tool"]["stestr"]
+            self.test_path = root.get("test_path", self.test_path)
+            self.top_dir = root.get("top_dir", self.top_dir)
+            self.parallel_class = root.get("parallel_class", self.parallel_class)
+            self.group_regex = root.get("group_regex", self.group_regex)
+
+    @classmethod
+    def load_from_file(cls, config):
+        """Load user-specified values from the various config files.
+
+        ConfigParser (.ini) and TOML are supported.
+        If a config file is specified, it is used, and fails on errors.
+        If no config file is specified, the order of precedence is as follows:
+        - .stestr.conf
+        - pyproject.toml with a valid [tool.stestr] section
+        - tox.ini with a valid [stestr] section
+
+        :param str config: The pathname of the config file to use
+        """
+        if os.path.isfile(config) or config != cls.DEFAULT_CONFIG_FILENAME:
+            return cls(config)
+        try:
+            return cls("pyproject.toml")
+        except (FileNotFoundError, KeyError):
+            return cls("tox.ini", section="stestr")
 
     def _sanitize_path(self, path):
         if os.sep == "\\":
@@ -114,22 +164,18 @@ def get_run_command(
         :rtype: test_processor.TestProcessorFixture
         """
 
-        if not test_path and self.parser.has_option(self.section, "test_path"):
-            test_path = self.parser.get(self.section, "test_path")
-        elif not test_path:
+        if not test_path and not self.test_path:
             sys.exit(
                 "No test_path can be found in either the command line "
                 "options nor in the specified config file {}.  Please "
                 "specify a test path either in the config file or via "
                 "the --test-path argument".format(self.config_file)
             )
-        if not top_dir and self.parser.has_option(self.section, "top_dir"):
-            top_dir = self.parser.get(self.section, "top_dir")
-        elif not top_dir:
+        if not top_dir and not self.top_dir:
             top_dir = "./"
 
-        test_path = self._sanitize_path(test_path)
-        top_dir = self._sanitize_path(top_dir)
+        test_path = self._sanitize_path(test_path or self.test_path)
+        top_dir = self._sanitize_path(top_dir or self.top_dir)
 
         stestr_python = sys.executable
         # let's try to be explicit, even if it means a longer set of ifs
@@ -160,16 +206,10 @@ def get_run_command(
         idoption = "--load-list $IDFILE"
         # If the command contains $IDOPTION read that command from config
         # Use a group regex if one is defined
-        if parallel_class:
-            group_regex = r"([^\.]*\.)*"
-        if (
-            not group_regex
-            and self.parser.has_option(self.section, "parallel_class")
-            and self.parser.getboolean(self.section, "parallel_class")
-        ):
+        if parallel_class or self.parallel_class:
             group_regex = r"([^\.]*\.)*"
-        if not group_regex and self.parser.has_option(self.section, "group_regex"):
-            group_regex = self.parser.get(self.section, "group_regex")
+        if not group_regex and self.group_regex:
+            group_regex = self.group_regex
         if group_regex:
 
             def group_callback(test_id, regex=re.compile(group_regex)):
diff --git a/stestr/tests/test_config_file.py b/stestr/tests/test_config_file.py
index 452ff31..a1a1e9d 100644
--- a/stestr/tests/test_config_file.py
+++ b/stestr/tests/test_config_file.py
@@ -9,10 +9,12 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
-
+import os
+import random
 from unittest import mock
 
 import ddt
+import fixtures
 
 from stestr import config_file
 from stestr.tests import base
@@ -24,7 +26,6 @@ class TestTestrConf(base.TestCase):
     def setUp(self, mock_ConfigParser):
         super().setUp()
         self._testr_conf = config_file.TestrConf(mock.sentinel.config_file)
-        self._testr_conf.parser = mock.Mock()
 
     @mock.patch.object(config_file.util, "get_repo_open")
     @mock.patch.object(config_file.test_processor, "TestProcessorFixture")
@@ -130,7 +131,8 @@ def test_get_run_command_parallel_class(self):
         self._check_get_run_command(parallel_class=True)
 
     def test_get_run_command_nogroup_regex_noparallel_class(self):
-        self._testr_conf.parser.has_option.return_value = False
+        self._testr_conf.parallel_class = False
+        self._testr_conf.group_regex = ""
         self._check_get_run_command(group_regex="", expected_group_callback=None)
 
     @ddt.data((".\\", ".\\\\"), ("a\\b\\", "a\\b\\\\"), ("a\\b", "a\\b"))
@@ -139,3 +141,62 @@ def test_get_run_command_nogroup_regex_noparallel_class(self):
     def test_sanitize_dir_win32(self, path, expected):
         sanitized = self._testr_conf._sanitize_path(path)
         self.assertEqual(expected, sanitized)
+
+    @mock.patch("os.path.isfile")
+    @mock.patch("stestr.config_file.TestrConf.__init__")
+    def test_load_from_file_user_specified(self, initializer, isfile):
+        # Test that user-specified config files are "one-and-done"
+        initializer.return_value = None
+        isfile.return_value = True
+        config_file.TestrConf.load_from_file("user.conf")
+        initializer.assert_called_once_with("user.conf")
+
+    @mock.patch("os.path.isfile")
+    @mock.patch("stestr.config_file.TestrConf.__init__")
+    def test_load_from_file_user_specified_fails(self, initializer, isfile):
+        # Test that user-specified config files that do not exist gives up
+        # immediately
+        initializer.return_value = None
+        initializer.side_effect = FileNotFoundError
+        isfile.return_value = False
+        self.assertRaises(
+            FileNotFoundError, config_file.TestrConf.load_from_file, "user.conf"
+        )
+        isfile.assert_called_once_with("user.conf")
+        initializer.assert_called_once_with("user.conf")
+
+    @mock.patch("os.path.isfile")
+    @mock.patch("stestr.config_file.TestrConf.__init__")
+    def test_load_from_file_toml_has_precedence(self, initializer, isfile):
+        # Test that tox.ini is ignored if a pyproject.toml config exists
+        initializer.return_value = None
+        isfile.return_value = False
+        config_file.TestrConf.load_from_file(".stestr.conf")
+        isfile.assert_called_once_with(".stestr.conf")
+        initializer.assert_called_once_with("pyproject.toml")
+
+    @mock.patch("os.path.isfile")
+    @mock.patch("stestr.config_file.TestrConf.__init__")
+    def test_load_from_file_ini_fallback(self, initializer, isfile):
+        initializer.return_value = None
+        # The only difference between "no config file" and "nothing defined
+        # in the config file" is the type of exception thrown; we'll make
+        # sure that, in aggregate, we test for both conditions
+        exc = random.choice([FileNotFoundError, KeyError])
+        initializer.side_effect = (exc, None)
+        isfile.return_value = False
+        config_file.TestrConf.load_from_file(".stestr.conf")
+        isfile.assert_called_once_with(".stestr.conf")
+        initializer.assert_has_calls(
+            [mock.call("pyproject.toml"), mock.call("tox.ini", section="stestr")]
+        )
+
+    @mock.patch.object(config_file.tomlkit, "load")
+    def test_toml_load(self, mock_toml):
+        tmpdir = self.useFixture(fixtures.TempDir()).path
+        file_path = os.path.join(tmpdir, "myfile.toml")
+        with open(file_path, "w"):
+            pass
+        self._testr_conf = config_file.TestrConf(file_path)
+        self._check_get_run_command()
+        mock_toml.return_value.__getitem__.assert_called_once_with("tool")