Repository: facebookincubator/ptr

Issue:
Add `setup.cfg` support
Allow people to configure tests to be ran from `setup.cfg` as well as `setup.py`

Discussion:

Patch:
diff --git a/mypy.ini b/mypy.ini
index 6a468f0..a6279a9 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -1,5 +1,5 @@
 [mypy]
-python_version = 3.5
+python_version = 3.6
 check_untyped_defs = True
 disallow_incomplete_defs = True
 disallow_untyped_decorators = True
diff --git a/ptr.py b/ptr.py
index d60320e..1649640 100755
--- a/ptr.py
+++ b/ptr.py
@@ -232,6 +232,12 @@ def _get_test_modules(base_path: Path, stats: Dict[str, int]) -> Dict[Path, Dict
 
     test_modules = {}  # type: Dict[Path, Dict]
     for setup_py in all_setup_pys:
+        # If a setup.cfg exists lets prefer it, if there is a [ptr] section
+        ptr_params = parse_setup_cfg(setup_py)
+        if ptr_params:
+            test_modules[setup_py] = ptr_params
+            continue
+
         with setup_py.open("r") as sp:
             setup_tree = ast.parse(sp.read())
 
@@ -564,6 +570,15 @@ async def create_venv(
     return venv_path
 
 
+def find_py_files(py_files: Set[str], base_dir: Path) -> None:
+    dirs = [d for d in base_dir.iterdir() if d.is_dir()]
+    py_files.update(
+        {str(x) for x in base_dir.iterdir() if x.is_file() and x.suffix == ".py"}
+    )
+    for directory in dirs:
+        find_py_files(py_files, directory)
+
+
 def find_setup_pys(
     base_path: Path, exclude_patterns: Set[str], follow_symlinks: bool = False
 ) -> Set[Path]:
@@ -593,6 +608,37 @@ def find_setup_pys(
     return setup_pys
 
 
+def parse_setup_cfg(setup_py: Path) -> Dict[str, Any]:
+    req_cov_key_strip = "required_coverage_"
+    ptr_params = {}  # type: Dict[str, Any]
+    setup_cfg = setup_py.parent / "setup.cfg"
+    if not setup_cfg.exists():
+        return ptr_params
+
+    cp = ConfigParser()
+    # Have configparser maintain key case - T484 error = callable
+    cp.optionxform = str  # type: ignore
+    cp.read(setup_cfg)
+    if "ptr" not in cp:
+        LOG.info("{} does not have a ptr section")
+        return ptr_params
+
+    # Create a setup.py like ptr_params to return
+    ptr_params["required_coverage"] = {}
+    for key, value in cp["ptr"].items():
+        if key.startswith(req_cov_key_strip):
+            key = key.strip(req_cov_key_strip)
+            ptr_params["required_coverage"][key] = int(value)
+        elif key.startswith("run_"):
+            ptr_params[key] = cp.getboolean("ptr", key)
+        elif key == "test_suite_timeout":
+            ptr_params[key] = cp.getint("ptr", key)
+        else:
+            ptr_params[key] = value
+
+    return ptr_params
+
+
 def print_test_results(
     test_results: Sequence[test_result], stats: Optional[Dict[str, int]] = None
 ) -> Dict[str, int]:
@@ -640,15 +686,6 @@ def print_test_results(
     return stats
 
 
-def find_py_files(py_files: Set[str], base_dir: Path) -> None:
-    dirs = [d for d in base_dir.iterdir() if d.is_dir()]
-    py_files.update(
-        {str(x) for x in base_dir.iterdir() if x.is_file() and x.suffix == ".py"}
-    )
-    for directory in dirs:
-        find_py_files(py_files, directory)
-
-
 async def run_tests(
     atonce: int,
     mirror: str,
diff --git a/setup.cfg.sample b/setup.cfg.sample
index 0f1ea18..1940e25 100644
--- a/setup.cfg.sample
+++ b/setup.cfg.sample
@@ -1,8 +1,9 @@
 [ptr]
 entry_point_module = ptr
 test_suite = ptr_tests
-test_suite_timeout = 300
-# TODO: Make this more ini like? - Big JSON will get U G L Y
-required_coverage = {"ptr.py": 84, "TOTAL": 90}
+test_suite_timeout = 120
+# `required_coverage_` will be stripped for use
+required_coverage_ptr.py = 84
+required_coverage_TOTAL = 90
 run_black = true
 run_mypy = true
diff --git a/setup.py b/setup.py
index b61df98..d75cb3f 100644
--- a/setup.py
+++ b/setup.py
@@ -16,7 +16,7 @@ ptr_params = {
     "test_suite": "ptr_tests",
     "test_suite_timeout": 120,
     # Relative path from setup.py to module (e.g. ptr == ptr.py)
-    "required_coverage": {"ptr.py": 84, "TOTAL": 90},
+    "required_coverage": {"ptr.py": 85, "TOTAL": 91},
     # Run black or not
     "run_black": True,
     # Run mypy or not


Test patch:
diff --git a/ptr_tests.py b/ptr_tests.py
index 687c49f..7c954bf 100644
--- a/ptr_tests.py
+++ b/ptr_tests.py
@@ -241,6 +241,18 @@ class TestPtr(unittest.TestCase):
         with self.assertRaises(SystemExit):
             ptr.main()
 
+    def test_parse_setup_cfg(self) -> None:
+        tmp_dir = Path(gettempdir())
+        setup_cfg = tmp_dir / "setup.cfg"
+        setup_py = tmp_dir / "setup.py"
+
+        with setup_cfg.open("w") as scp:
+            scp.write(ptr_tests_fixtures.SAMPLE_SETUP_CFG)
+
+        self.assertEqual(
+            ptr.parse_setup_cfg(setup_py), ptr_tests_fixtures.EXPECTED_TEST_PARAMS
+        )
+
     @patch("ptr.print")  # noqa
     def test_print_test_results(self, mock_print: Mock) -> None:
         stats = ptr.print_test_results(ptr_tests_fixtures.EXPECTED_COVERAGE_RESULTS)
diff --git a/ptr_tests_fixtures.py b/ptr_tests_fixtures.py
index b5d1d62..4f08086 100644
--- a/ptr_tests_fixtures.py
+++ b/ptr_tests_fixtures.py
@@ -24,7 +24,7 @@ EXPECTED_TEST_PARAMS = {
     "entry_point_module": "ptr",
     "test_suite": "ptr_tests",
     "test_suite_timeout": 120,
-    "required_coverage": {"ptr.py": 84, "TOTAL": 90},
+    "required_coverage": {"ptr.py": 85, "TOTAL": 91},
     "run_black": True,
     "run_mypy": True,
 }
@@ -148,6 +148,17 @@ setup(
 )
 """
 
+SAMPLE_SETUP_CFG = """\
+[ptr]
+entry_point_module = ptr
+test_suite = ptr_tests
+test_suite_timeout = 120
+required_coverage_ptr.py = 85
+required_coverage_TOTAL = 91
+run_black = true
+run_mypy = true
+"""
+
 SAMPLE_SETUP_PY_PTR = {
     "test_suite": "coop.tests.base",
     "test_suite_timeout": 60,