fbcode_builder: fix dead retry loop in ctest test runner

Summary:
The ctest retry loop in BuilderBase test_results_for_retry was calling
self._check_cmd(args, allow_fail=True) and capturing its return code,
but _check_cmd raises RuntimeError whenever its inner _run_cmd returns
non-zero — independently of allow_fail (which is only forwarded to
_run_cmd to suppress the lower-level raise). On the first ctest
failure the loop therefore raised out before ever reaching the
--rerun-failed retry path: the retry was dead code.

Switch the call to _run_cmd so a non-zero exit is returned and the
retry loop actually runs. This also lets us drop the int|None typing
dance (retcode could only ever be None when a never-thrown exception
ate the assignment) and the corresponding 'is not None' guard. The
FIXME asking what the loop accomplishes is removed: the answer is
'rerun failed tests up to retry times', which is now what it does.

Reviewed By: bigfootjon

Differential Revision: D103767928

fbshipit-source-id: 466cb4ce598acbb2458fe4327e5d322fe12cffbd

Author: r-barnes

build/fbcode_builder/getdeps/builder.py

@@ -1330,10 +1330,9 @@ def list_tests() -> list[dict[str, object]]:
                 args += ["--timeout", str(timeout)]
 
             count: int = 0
-            retcode: int | None = -1
+            retcode: int = -1
             while count <= retry:
-                # FIXME: What is this trying to accomplish? Should it fail on first or >=1 errors?
-                retcode = self._check_cmd(
+                retcode = self._run_cmd(
                     args, env=env, use_cmd_prefix=use_cmd_prefix, allow_fail=True
                 )
 
@@ -1343,7 +1342,7 @@ def list_tests() -> list[dict[str, object]]:
                     # Only add this option in the second run.
                     args += ["--rerun-failed"]
                 count += 1
-            if retcode is not None and retcode != 0:
+            if retcode != 0:
                 # Allow except clause in getdeps.main to catch and exit gracefully
                 # This allows non-testpilot runs to fail through the same logic as failed testpilot runs, which may become handy in case if post test processing is needed in the future
                 raise subprocess.CalledProcessError(retcode, args)