Repository: facebookincubator/ptr

Issue:
Add an `disabled` setup.py/cfg option + --force CLI option
## How is `ptr` not doing what you expect?
Today I have to comment out tests if I don't want them to run in large repo crawling CI job. It would be nice to have them "disabled" by default, but have a CLI option to ignore this flag for individual testing + fixing.

## What is your suggestion to make this better?
Add to the code logic to not run `auto_run=False` setup.cfg|py files.

Discussion:

Patch:
diff --git a/ci.py b/ci.py
index af375a9..3804be7 100644
--- a/ci.py
+++ b/ci.py
@@ -67,7 +67,15 @@ def integration_test() -> int:
     print("Running `ptr` integration tests (aka run itself)", file=stderr)
 
     stats_file = Path(gettempdir()) / "ptr_ci_stats"
-    ci_cmd = ["python", "ptr.py", "-d", "--print-cov", "--stats-file", str(stats_file)]
+    ci_cmd = [
+        "python",
+        "ptr.py",
+        "-d",
+        "--print-cov",
+        "--run-disabled",
+        "--stats-file",
+        str(stats_file),
+    ]
     if "VIRTUAL_ENV" in environ:
         ci_cmd.extend(["--venv", environ["VIRTUAL_ENV"]])
 
diff --git a/ptr.py b/ptr.py
index 7ae1a05..5ee0624 100755
--- a/ptr.py
+++ b/ptr.py
@@ -295,7 +295,9 @@ def _generate_pyre_cmd(
     return (str(pyre_exe), "--source-directory", str(module_dir), "check")
 
 
-def _get_test_modules(base_path: Path, stats: Dict[str, int]) -> Dict[Path, Dict]:
+def _get_test_modules(
+    base_path: Path, stats: Dict[str, int], run_disabled: bool
+) -> Dict[Path, Dict]:
     get_tests_start_time = time()
     all_setup_pys = find_setup_pys(
         base_path,
@@ -306,11 +308,18 @@ def _get_test_modules(base_path: Path, stats: Dict[str, int]) -> Dict[Path, Dict
     stats["total.setup_pys"] = len(all_setup_pys)
 
     test_modules = {}  # type: Dict[Path, Dict]
-    for setup_py in all_setup_pys:
+    for setup_py in all_setup_pys:  # pylint: disable=R1702
+        disabled_err_msg = "Not running {} as ptr is disabled via config".format(
+            setup_py
+        )
         # 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
+            if ptr_params.get("disabled", False) and not run_disabled:
+                LOG.info(disabled_err_msg)
+                stats["total.disabled"] += 1
+            else:
+                test_modules[setup_py] = ptr_params
             continue
 
         with setup_py.open("r", encoding="utf8") as sp:
@@ -325,7 +334,11 @@ def _get_test_modules(base_path: Path, stats: Dict[str, int]) -> Dict[Path, Dict
                         LOG.debug("Found ptr_params in {}".format(setup_py))
                         ptr_params = ast.literal_eval(node.value)
                         if "test_suite" in ptr_params:
-                            test_modules[setup_py] = ptr_params
+                            if ptr_params.get("disabled", False) and not run_disabled:
+                                LOG.info(disabled_err_msg)
+                                stats["total.disabled"] += 1
+                            else:
+                                test_modules[setup_py] = ptr_params
                         else:
                             LOG.info(
                                 "{} does not have a suite. Nothing to run".format(
@@ -780,7 +793,7 @@ def parse_setup_cfg(setup_py: Path) -> Dict[str, Any]:
         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_"):
+        elif key.startswith("run_") or key == "disabled":
             ptr_params[key] = cp.getboolean("ptr", key)
         elif key == "test_suite_timeout":
             ptr_params[key] = cp.getint("ptr", key)
@@ -816,10 +829,11 @@ def print_test_results(
     # TODO: Hardcode some workaround to ensure Windows always prints UTF8
     # https://github.com/facebookincubator/ptr/issues/34
     print(
-        "✅ PASS: {}\n❌ FAIL: {}\n⌛️ TIMEOUT: {}\n💩 TOTAL: {}\n".format(
+        "✅ PASS: {}\n❌ FAIL: {}\n️⌛ TIMEOUT: {}\n🔒 DISABLED: {}\n💩 TOTAL: {}\n".format(
             stats["total.passes"],
             stats["total.fails"],
             stats["total.timeouts"],
+            stats["total.disabled"],
             stats["total.test_suites"],
         )
     )
@@ -921,12 +935,13 @@ async def async_main(
     venv: str,
     venv_keep: bool,
     print_cov: bool,
+    run_disabled: bool,
     force_black: bool,
     stats_file: str,
     venv_timeout: float,
 ) -> int:
     stats = defaultdict(int)  # type: Dict[str, int]
-    tests_to_run = _get_test_modules(base_path, stats)
+    tests_to_run = _get_test_modules(base_path, stats, run_disabled)
     if not tests_to_run:
         LOG.error(
             "{} has no setup.py files with unit tests defined. Exiting".format(
@@ -1004,6 +1019,11 @@ def main() -> None:
         type=float,
         help="Seconds between status update on test running [Default: Disabled]",
     )
+    parser.add_argument(
+        "--run-disabled",
+        action="store_true",
+        help="Force any disabled tests suites to run",
+    )
     parser.add_argument(
         "--stats-file",
         default=str(default_stats_file),
@@ -1033,6 +1053,7 @@ def main() -> None:
                     args.venv,
                     args.keep_venv,
                     args.print_cov,
+                    args.run_disabled,
                     args.force_black,
                     args.stats_file,
                     args.venv_timeout,
diff --git a/setup.py b/setup.py
index e15b306..d380fbb 100644
--- a/setup.py
+++ b/setup.py
@@ -12,6 +12,8 @@ from setuptools import setup
 
 # Specific Python Test Runner (ptr) params for Unit Testing Enforcement
 ptr_params = {
+    # Disable auto running if found - Requires --run-disabled to run
+    "disabled": True,
     # Where mypy will run to type check your program
     "entry_point_module": "ptr",
     # Base Unittest File


Test patch:
diff --git a/ptr_tests.py b/ptr_tests.py
index 884dfcd..39afaeb 100644
--- a/ptr_tests.py
+++ b/ptr_tests.py
@@ -130,7 +130,7 @@ class TestPtr(unittest.TestCase):
     @patch("ptr.run_tests", async_none)
     @patch("ptr._get_test_modules")
     def test_async_main(self, mock_gtm: Mock) -> None:
-        args = [1, Path("/"), "mirror", 1, "venv", True, True, False, "stats", 30]
+        args = [1, Path("/"), "mirror", 1, "venv", True, True, True, False, "stats", 30]
         mock_gtm.return_value = False
         self.assertEqual(
             self.loop.run_until_complete(ptr.async_main(*args)), 1  # pyre-ignore
@@ -283,7 +283,7 @@ class TestPtr(unittest.TestCase):
     def test_get_test_modules(self) -> None:
         base_path = Path(__file__).parent
         stats = defaultdict(int)  # type: Dict[str, int]
-        test_modules = ptr._get_test_modules(base_path, stats)
+        test_modules = ptr._get_test_modules(base_path, stats, True)
         self.assertEqual(
             test_modules[base_path / "setup.py"],
             ptr_tests_fixtures.EXPECTED_TEST_PARAMS,
diff --git a/ptr_tests_fixtures.py b/ptr_tests_fixtures.py
index 884d438..d278888 100644
--- a/ptr_tests_fixtures.py
+++ b/ptr_tests_fixtures.py
@@ -23,7 +23,9 @@ class FakeEventLoop:
         return 0
 
 
+# Disabled is set as we --run-disabled the run in CI
 EXPECTED_TEST_PARAMS = {
+    "disabled": True,
     "entry_point_module": "ptr",
     "test_suite": "ptr_tests",
     "test_suite_timeout": 120,
@@ -187,8 +189,10 @@ setup(
 )
 """
 
+# Disabled is set as we --run-disabled the run in CI
 SAMPLE_SETUP_CFG = """\
 [ptr]
+disabled = true
 entry_point_module = ptr
 test_suite = ptr_tests
 test_suite_timeout = 120