Skip to content

Commit b692119

Browse files
benoit-nexthopmeta-codesync[bot]
authored andcommitted
Build missing test binaries automatically when running tests
Summary: **Pre-submission checklist** - [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install` - [x] `pre-commit run` When building only a specific cmake target to save time, getdeps removes the "built marker” under `$builddir/installed/fboss/.built-by-getdeps`, which makes it impossible to subsequently run the unit tests as without this file getdeps bails out with `project fboss has not been built`. What’s more, because of a missing `return` the exit code was 0, which was confusing. This fix ensures that the return code is propagated properly and also improves the cmake test runner to automatically detect missing executables and build them on the fly. This will not rebuild the test binaries if they've already been built and are merely stale (because the source code has changed) but that was already the prior behavior. At least now it's possible to rebuild a single cmake test target and re-run the tests in one shot. X-link: facebook/fboss#722 Reviewed By: bigfootjon Differential Revision: D93524014 Pulled By: joseph5wu fbshipit-source-id: d58055430e181f4efa6901985b7b2e7de4626a11
1 parent abbd1b1 commit b692119

File tree

1 file changed

+88
-0
lines changed

1 file changed

+88
-0
lines changed

build/fbcode_builder/getdeps/builder.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
import glob
1010
import json
1111
import os
12+
import os.path
1213
import pathlib
14+
import re
1315
import shutil
1416
import stat
1517
import subprocess
@@ -922,6 +924,80 @@ def _build(self, reconfigure: bool) -> None:
922924
env=env,
923925
)
924926

927+
def _build_targets(self, targets: typing.Sequence[str]) -> None:
928+
"""Build one or more cmake targets in parallel.
929+
930+
Args:
931+
targets: Sequence of target names (strings) to build
932+
"""
933+
if not targets:
934+
return
935+
936+
env = self._compute_env()
937+
cmake = path_search(env, "cmake")
938+
if cmake is None:
939+
raise RuntimeError("unable to find cmake")
940+
941+
# Build all targets in a single cmake invocation for better parallelism
942+
cmd = [
943+
cmake,
944+
"--build",
945+
self.build_dir,
946+
]
947+
948+
# Add all targets
949+
for target in targets:
950+
cmd.extend(["--target", target])
951+
952+
cmd.extend(
953+
[
954+
"--config",
955+
self.build_opts.build_type,
956+
"-j",
957+
str(self.num_jobs),
958+
]
959+
)
960+
961+
self._check_cmd(cmd, env=env)
962+
963+
def _get_missing_test_executables(
964+
self, test_filter: Optional[str], env: Env, ctest: Optional[str]
965+
) -> typing.Set[str]:
966+
"""Discover which test executables are missing for the given filter.
967+
Returns a set of missing executable basenames (without path)."""
968+
if ctest is None:
969+
return set()
970+
971+
# Run ctest -N (show tests without running) with the filter to see which tests match
972+
cmd = [ctest, "-N"]
973+
if test_filter:
974+
cmd += ["-R", test_filter]
975+
976+
try:
977+
output = subprocess.check_output(
978+
cmd,
979+
env=dict(env.items()),
980+
cwd=self.build_dir,
981+
stderr=subprocess.STDOUT,
982+
text=True,
983+
)
984+
except subprocess.CalledProcessError as e:
985+
# If ctest fails, it might be because executables don't exist yet
986+
# Parse the error output to find the missing executables
987+
output = e.output
988+
989+
# Parse output to find missing executable paths
990+
# Look for lines like "Could not find executable /path/to/test_binary"
991+
missing_executables = set()
992+
for line in output.split("\n"):
993+
match = re.search(r"Could not find executable (.+)", line)
994+
if match:
995+
exe_path = match.group(1)
996+
exe_name = os.path.basename(exe_path)
997+
missing_executables.add(exe_name)
998+
999+
return missing_executables
1000+
9251001
def run_tests(
9261002
self,
9271003
schedule_type,
@@ -936,6 +1012,18 @@ def run_tests(
9361012
ctest = path_search(env, "ctest")
9371013
cmake = path_search(env, "cmake")
9381014

1015+
# Build only the missing test executables needed for the given filter.
1016+
# This is especially important for LocalDirFetcher projects (like fboss)
1017+
# where the build marker gets removed when building specific cmake targets.
1018+
missing_test_executables = self._get_missing_test_executables(
1019+
test_filter, env, ctest
1020+
)
1021+
if missing_test_executables:
1022+
sorted_executables = sorted(missing_test_executables)
1023+
print(f"Building missing test executables: {', '.join(sorted_executables)}")
1024+
# Build all missing executables in one cmake invocation for better parallelism
1025+
self._build_targets(sorted_executables)
1026+
9391027
def require_command(path: Optional[str], name: str) -> str:
9401028
if path is None:
9411029
raise RuntimeError("unable to find command `{}`".format(name))

0 commit comments

Comments
 (0)