Run tests as their own Sandcastle jobs

Summary:
This may or may not be a good idea... please look at the `cinderx_ci-...` runs on this diff and see how you feel about this. With this diff I saw my sandcastle jobs as being throttled - I'm not sure if Sandcastle will adapt to the fact that these run quickly over time. The "Duration" for these on the diff page also seems to include queueing time, so you can see they actually run much faster.

This splits our test cases out to run one by one. Now if a test is failing we'll have just a single failure instead of having an entire opaque test run failing.

We can't read the list of tests at build time so we now have a cached list of known tests. There's a new test which tests that the tests are up to date, and if they're not `TestScripts/test_test_defs.sh` can be run and the resulting `/tmp/tests.txt` can be copied over `cinderx/tests.bzl`.

The test runners needed to be updated to have a new `--list` option to print out all of the tests they discovered. There also were some small updates so that the individual tests would successfully run and pass:
* We need to set sys.path on 3.10 in the worker like we do on 3.12+
* When running an individual module on 3.10 we need to fallback to `_runtest_inner2` so that it can run 'test_main` to setup the module. This also means we need to get rid of it's propensity to pre-pend `test.` if a test name doesn't start with that so that we can run `test_cinderx` tests.
* If no tests are run we report success (because our filters can now filter down to 0 tests).

Probably the most annoying thing about this is when we're onboarding a new version we don't get a nice concise list of test failures to update a failures script. I'm curious what people think about that... we could always have some easy-to-enable thing which runs all of the tests.

This also enables base revision retries now that they should be a little faster as they target individual tests.

Reviewed By: alexmalyshev

Differential Revision: D85780300

fbshipit-source-id: 94168f134ba98d62088eaa8c80f16f3f2cc7231a

Author: DinoV

cinderx/TestScripts/cinder_test_runner310.py

@@ -31,8 +31,8 @@
 import types
 import unittest
 
+from importlib.util import find_spec
 from pathlib import Path
-
 from typing import Dict, IO, Iterable, List, Optional, Set, Tuple
 
 from cinderx.test_support import get_cinderjit_xargs, is_asan_build
@@ -62,7 +62,9 @@
 from test import support
 from test.libregrtest.cmdline import Namespace
 from test.libregrtest.main import Regrtest
+
 from test.libregrtest.runtest import (
+    _runtest_inner2,
     ChildError,
     findtestdir,
     findtests,
@@ -156,6 +158,10 @@ def run(self, ns: types.SimpleNamespace) -> None:
         t.set_temp_dir()
         test_cwd = t.create_temp_dir()
         setup_tests(t.ns)
+        import test.libregrtest.runtest as rt
+
+        rt._runtest_inner2 = _patched_runtest_inner2
+        rt.get_abs_module = lambda ns, name: name
         # Run the tests in a context manager that temporarily changes the CWD to a
         # temporary and writable directory.  If it's not possible to create or
         # change the CWD, the original CWD will be used.  The original CWD is
@@ -321,6 +327,45 @@ def _setupCinderIgnoredTests(ns: Namespace, use_rr: bool) -> Tuple[List[str], Se
     return list(stdtest_set), nottests
 
 
+@staticmethod
+def list_tests(ns: Namespace, use_rr: bool) -> None:
+    cinderx_dir = get_cinderx_dir()
+    test_cinderx_dir = get_test_cinderx_dir(cinderx_dir)
+
+    # Added to sys.path for test modules.  `test_cinderx_dir.parent` is just
+    # `cinderx_dir` in the Git repository, but it can be different for
+    # internal builds.
+    ns.testdir = str(test_cinderx_dir.parent)
+
+    setup_tests(ns)
+    test_filters = _setupCinderIgnoredTests(ns, use_rr)
+
+    cinderx_dir = get_cinderx_dir()
+    test_cinderx_dir = get_test_cinderx_dir(cinderx_dir)
+
+    stdtest, nottests = test_filters
+
+    # Initial set of tests are the core Python/Cinder ones.
+    tests = ["test." + t for t in findtests(None, stdtest, nottests)]
+
+    # Add CinderX tests
+    cinderx_tests = findtests(str(test_cinderx_dir), [], nottests)
+    tests.extend("test_cinderx." + t for t in cinderx_tests if not t == "test_compiler")
+
+    # Split the compiler tests into their subtests so they run faster
+    compiler_tests = findtests(str(test_cinderx_dir) + "/test_compiler", [], nottests)
+    tests.extend(
+        "test_cinderx.test_compiler." + t for t in compiler_tests if t != "test_static"
+    )
+
+    # Also split the static Python tests, they don't start with "test_" so we
+    # need to manually discover them, and exclude
+    testdir = findtestdir(str(test_cinderx_dir) + "/test_compiler/test_static")
+    tests.extend(get_cinderx_static_tests(testdir))
+
+    return tests
+
+
 class MultiWorkerCinderRegrtest(Regrtest):
     def __init__(
         self,
@@ -476,21 +521,10 @@ def _save_recording_metadata(self, replay_infos: List[ReplayInfo]) -> None:
     def _main(self, tests, kwargs):
         self.ns.fail_env_changed = True
 
-        cinderx_dir = get_cinderx_dir()
-        test_cinderx_dir = get_test_cinderx_dir(cinderx_dir)
-
-        # Added to sys.path for test modules.  `test_cinderx_dir.parent` is just
-        # `cinderx_dir` in the Git repository, but it can be different for
-        # internal builds.
-        self.ns.testdir = str(test_cinderx_dir.parent)
-
-        setup_tests(self.ns)
-
-        test_filters = _setupCinderIgnoredTests(self.ns, self._use_rr)
-
         if tests is None:
-            self._selectDefaultCinderTests(test_filters, test_cinderx_dir)
+            self.tests = list_tests(self.ns, self._use_rr)
         else:
+            _setupCinderIgnoredTests(self.ns, self._use_rr)
             self.find_tests(tests)
 
         replay_infos = self.run_tests()
@@ -519,37 +553,6 @@ def _main(self, tests, kwargs):
                 sys.exit(3)
         sys.exit(0)
 
-    def _selectDefaultCinderTests(
-        self, test_filters: Tuple[List[str], Set[str]], test_cinderx_dir: Path
-    ) -> None:
-        stdtest, nottests = test_filters
-
-        # Initial set of tests are the core Python/Cinder ones.
-        tests = ["test." + t for t in findtests(None, stdtest, nottests)]
-
-        # Add CinderX tests
-        cinderx_tests = findtests(str(test_cinderx_dir), [], nottests)
-        tests.extend(
-            "test_cinderx." + t for t in cinderx_tests if not t == "test_compiler"
-        )
-
-        # Spilt the compiler tests into their subtests so they run faster
-        compiler_tests = findtests(
-            str(test_cinderx_dir) + "/test_compiler", [], nottests
-        )
-        tests.extend(
-            "test_cinderx.test_compiler." + t
-            for t in compiler_tests
-            if t != "test_static"
-        )
-
-        # Also split the static Python tests, they don't start with "test_" so we
-        # need to manually discover them, and exclude
-        testdir = findtestdir(str(test_cinderx_dir) + "/test_compiler/test_static")
-        tests.extend(get_cinderx_static_tests(testdir))
-
-        self.selected = tests
-
     def _writeResultsToScuba(self) -> None:
         template = {
             "int": {
@@ -593,8 +596,15 @@ def _writeResultsToScuba(self) -> None:
 def _patched_runtest_inner2(ns: Namespace, tests_name: str) -> bool:
     import test.libregrtest.runtest as runtest
 
+    if find_spec(tests_name) is not None:
+        # if we have a module fallback to the existing _runtest_inner2, it
+        # does some additioanl setup like calling test_main that we can't
+        # handle for non-module tests.
+        return _runtest_inner2(ns, tests_name)
+
     loader = unittest.TestLoader()
     tests = loader.loadTestsFromName(tests_name, None)
+
     for error in loader.errors:
         print(error, file=sys.stderr)
     if loader.errors:
@@ -687,6 +697,7 @@ def force_dots_output(self, *args, **kwargs):
 
 
 def worker_main(args):
+    sys.path.insert(0, str(get_cinderx_dir() / "PythonLib"))
     ns_dict = json.loads(args.ns)
     ns = types.SimpleNamespace(**ns_dict)
     with MessagePipe(args.cmd_fd, args.result_fd) as pipe:
@@ -752,7 +763,6 @@ def dispatcher_main(args):
         with tempfile.NamedTemporaryFile(
             delete=False, mode="w+t", dir=CINDER_RUNNER_LOG_DIR
         ) as logfile:
-            print(f"Using scheduling log file {logfile.name}")
             test_runner = MultiWorkerCinderRegrtest(
                 logfile,
                 args.log_to_scuba,
@@ -763,6 +773,15 @@ def dispatcher_main(args):
                 args.recording_metadata_path,
                 args.no_retry_on_test_errors,
             )
+            if args.list:
+                sys.argv[1:] = args.rest[1:]
+                test_runner.parse_args({})
+                tests = list_tests(test_runner.ns, args.use_rr)
+                tests.sort()
+                print(json.dumps(tests, indent=0))
+                sys.exit(0)
+
+            print(f"Using scheduling log file {logfile.name}")
             test_runner.num_workers = args.num_workers
             print(f"Spawning {test_runner.num_workers} workers")
             # Put any args we didn't care about into sys.argv for
@@ -874,6 +893,12 @@ def replay_main(args):
         action="append",
         help="The name of a test to run (e.g. `test_math`). Can be supplied multiple times.",
     )
+    dispatcher_parser.add_argument(
+        "-l",
+        "--list",
+        action="store_true",
+        help="List tests and exit.",
+    )
     dispatcher_parser.add_argument(
         "-R",
         "--huntrleaks",

cinderx/TestScripts/cinder_test_runner312.py

@@ -329,6 +329,40 @@ def _computeSkipTests(huntrleaks, use_rr=False) -> Tuple[Set[str], Set[str]]:
     return skip_modules, skip_patterns
 
 
+def _select_tests(exclude: Set[str]) -> List[str]:
+    # Initial set of tests are the core Python ones.
+    tests = libregrtest_findtests.findtests(
+        exclude=exclude,
+        base_mod="test",
+        split_test_dirs={"test." + d for d in libregrtest_findtests.SPLITTESTDIRS},
+    )
+
+    # Add CinderX tests
+    cinderx_tests = libregrtest_findtests.findtests(
+        testdir=get_test_cinderx_dir(),
+        exclude=exclude,
+        split_test_dirs=CINDERX_SPLIT_TEST_DIRS,
+        base_mod="test_cinderx",
+    )
+    tests.extend(cinderx_tests)
+
+    # findtests won't discover the static tests that don't start with test_, so manually
+    # add those (it would find just test_static if we didn't split on that, but we want
+    # to parallelize all of the static tests)
+    testdir = libregrtest_findtests.findtestdir(
+        get_test_cinderx_dir() / Path("test_compiler/test_static")
+    )
+    tests.extend(get_cinderx_static_tests(testdir))
+
+    return tests
+
+
+def list_tests(hunterleaks, use_rr):
+    skip_modules, skip_patterns = _computeSkipTests(hunterleaks, use_rr)
+
+    return _select_tests(skip_modules)
+
+
 class MultiWorkerCinderRegrtest:
     def __init__(
         self,
@@ -375,7 +409,7 @@ def __init__(
         skip_modules, skip_patterns = _computeSkipTests(self._huntrleaks, self._use_rr)
 
         if tests is None:
-            tests = self._selectTests(skip_modules)
+            tests = _select_tests(skip_modules)
 
         extra_opts = {}
         if sys.version_info >= (3, 14):
@@ -622,6 +656,8 @@ def run(self):
             # True, True => fail_env_changed, fail_rerun
             if self._results.bad:
                 self.write_new_failures(starting_bad, missing_failures)
+            if self._results.no_tests_run():
+                sys.exit(0)
             sys.exit(self._results.get_exitcode(False, False))
         sys.exit(0)
 
@@ -654,8 +690,12 @@ def update_expected_failures(self) -> tuple[list[str], list[str]]:
         starting_bad = list(self._results.bad)
         starting_bad.sort()
         missing_failures = []
+        executed = self._results.get_executed()
         for expected_failure in self._expected_failures:
-            if expected_failure not in self._results.bad:
+            if (
+                expected_failure not in self._results.bad
+                and expected_failure in executed
+            ):
                 missing_failures.append(expected_failure)
             else:
                 # Failure was expected, move to the skipped list
@@ -666,33 +706,6 @@ def update_expected_failures(self) -> tuple[list[str], list[str]]:
 
         return starting_bad, missing_failures
 
-    def _selectTests(self, exclude: Set[str]) -> List[str]:
-        # Initial set of tests are the core Python ones.
-        tests = libregrtest_findtests.findtests(
-            exclude=exclude,
-            base_mod="test",
-            split_test_dirs={"test." + d for d in libregrtest_findtests.SPLITTESTDIRS},
-        )
-
-        # Add CinderX tests
-        cinderx_tests = libregrtest_findtests.findtests(
-            testdir=get_test_cinderx_dir(),
-            exclude=exclude,
-            split_test_dirs=CINDERX_SPLIT_TEST_DIRS,
-            base_mod="test_cinderx",
-        )
-        tests.extend(cinderx_tests)
-
-        # findtests won't discover the static tests that don't start with test_, so manually
-        # add those (it would find just test_static if we didn't split on that, but we want
-        # to parallelize all of the static tests)
-        testdir = libregrtest_findtests.findtestdir(
-            get_test_cinderx_dir() / Path("test_compiler/test_static")
-        )
-        tests.extend(get_cinderx_static_tests(testdir))
-
-        return tests
-
 
 # TASK(T184566736) Remove this work around for a bug in Buck2 which causes
 # us to be the parent of fire-and-forget logging processes.
@@ -856,6 +869,12 @@ def worker_main(args):
 
 
 def dispatcher_main(args):
+    if args.list:
+        tests = list_tests(args.huntrleaks, args.use_rr)
+        tests.sort()
+        print(json.dumps(tests, indent=0))
+        sys.exit(0)
+
     sys.path.insert(0, str(get_cinderx_dir() / "PythonLib"))
     libregrtest_setup.setup_process()
     pathlib.Path(CINDER_RUNNER_LOG_DIR).mkdir(parents=True, exist_ok=True)
@@ -924,7 +943,7 @@ def main():
         default=mem_limit_default,
     )
 
-    subparsers = parser.add_subparsers()
+    subparsers = parser.add_subparsers(required=False)
 
     worker_parser = subparsers.add_parser("worker")
     worker_parser.add_argument(
@@ -987,6 +1006,13 @@ def main():
         action="append",
         help="The name of a test to run (e.g. `test_math`). Can be supplied multiple times.",
     )
+    dispatcher_parser.add_argument(
+        "-l",
+        "--list",
+        action="store_true",
+        default=False,
+        help="List tests and exit.",
+    )
     dispatcher_parser.add_argument(
         "--replay",
         type=str,

cinderx/TestScripts/test_test_defs.sh

@@ -0,0 +1,28 @@
+#!/bin/bash
+
+# Verify that the tests.bzl file is up to date and accounts for all of the CPython and cinder tests.
+# If the tests diff you can re-run this and cp /tmp/tests.txt to cinderx/tests.bzl
+
+set -e
+
+cd "$(dirname "$(readlink -f "$0")")"/../
+
+buck run fbcode//cinderx:ctl3.10 > /tmp/3.10.txt
+buck run fbcode//cinderx:ctl3.12 > /tmp/3.12.txt
+buck run fbcode//cinderx:ctl3.14 > /tmp/3.14.txt
+
+{
+    echo "TESTS = {"
+    echo \"3.10\":
+    cat /tmp/3.10.txt
+    echo ","
+    echo \"3.12\":
+    cat /tmp/3.12.txt
+    echo ","
+    echo \"3.14\":
+    cat /tmp/3.14.txt
+    echo "}"
+} > /tmp/tests.txt
+pyfmt /tmp/tests.txt
+
+diff /tmp/tests.txt tests.bzl