PSMDB-1880 Automatically set RUNFILES_DIR for unittests executed via resmoke #1658
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds automatic RUNFILES_DIR environment variable configuration for Bazel-built C++ unit tests executed via resmoke. This eliminates the need to manually specify RUNFILES_DIR when running Bazel tests.
Key Changes:
- Adds automatic detection and configuration of RUNFILES_DIR for Bazel tests
- Implements symlink resolution to locate the correct runfiles directory
- Only applies to executables in bazel-bin directories and respects existing RUNFILES_DIR settings
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if this looks like a Bazel binary (in bazel-bin directory) | ||
| executable_path = os.path.abspath(self.program_executable) | ||
| self.logger.debug("Executable path is %s", executable_path) | ||
| if "bazel-bin" not in executable_path: |
There was a problem hiding this comment.
The string matching check for "bazel-bin" is fragile and could incorrectly match paths that happen to contain this substring (e.g., "/home/my-bazel-bin-backup/test"). Consider using a more robust path component check, such as checking if "bazel-bin" is an actual directory component in the path.
| if "bazel-bin" not in executable_path: | |
| normalized_executable_path = os.path.normpath(executable_path) | |
| path_components = normalized_executable_path.split(os.sep) | |
| if "bazel-bin" not in path_components: |
| ) | ||
| return # Not a Bazel binary, skip | ||
|
|
||
| # Check if executable path is a soft link and resolve it into resolved_path |
There was a problem hiding this comment.
Corrected "soft link" to "symlink" for consistency with the terminology used elsewhere in this function (see line 54).
| # Check if executable path is a soft link and resolve it into resolved_path | |
| # Check if executable path is a symlink and resolve it into resolved_path |
| # Check if .runfiles directory exists next to the binary but take binary name from the symlink name | ||
| # Take base dir from resolved_path | ||
| base_dir = os.path.dirname(resolved_path) | ||
| # Take binary name from executable_path | ||
| binary_name = os.path.basename(executable_path) |
There was a problem hiding this comment.
The logic for constructing the runfiles directory path has a potential issue. When the executable is a symlink, this code takes the base directory from the resolved path but the binary name from the symlink path. If the symlink and its target are in different directories, this could construct an incorrect runfiles path. For example, if executable_path is "/path/to/symlink/test" pointing to "/different/path/actual_binary", the code would look for "/different/path/test.runfiles" which may not exist. Consider using either both components from the resolved path or both from the symlink path for consistency.
| # Check if .runfiles directory exists next to the binary but take binary name from the symlink name | |
| # Take base dir from resolved_path | |
| base_dir = os.path.dirname(resolved_path) | |
| # Take binary name from executable_path | |
| binary_name = os.path.basename(executable_path) | |
| # Check if .runfiles directory exists next to the (possibly resolved) binary | |
| # Take both base dir and binary name from resolved_path for consistency | |
| base_dir = os.path.dirname(resolved_path) | |
| binary_name = os.path.basename(resolved_path) |
There was a problem hiding this comment.
This is exactly what was intended here. The symlinked binary has different name however the .runfile directory name matches the symlink's base name
…resmoke
So far we have single test affected by this change: extensions_api_test
With this change we can succefully run it this way:
buildscripts/resmoke.py run --suite unittests bazel-bin/install/bin/extensions_api_test
So far we have single test affected by this change: extensions_api_test With this change we can succefully run it this way: