Fix dtest -f relative paths under bazel run

[tameware]

Summary

• Resolves literal relative -f paths (e.g. hands/list1.txt) via BUILD_WORKING_DIRECTORY / BUILD_WORKSPACE_DIRECTORY and relative to the dtest binary, so bazelisk run //library/tests:dtest -- -f hands/list1.txt works.
• Numeric list shorthand (-f 1 → hands/list1.txt) behavior is unchanged; docs and error text updated to match.
• Adds unit coverage for literal relative resolution under bazel env dirs and binary-relative fallback.

Fixes #328

Test plan

• bazelisk test //library/tests:args_test
• bazelisk run //library/tests:dtest -- -f hands/list1.txt -s calc
• Confirm bazel-bin/library/tests/dtest -f hands/list1.txt still works from the repo root

Made with Cursor

[Copilot]

Pull request overview

Updates dtest argument handling so literal relative -f paths resolve correctly when running under bazelisk run, using Bazel-provided environment directories and an argv0-based fallback to locate the workspace hands/ files.

Changes:

• Extends -f/--file resolution to retry literal relative paths under BUILD_WORKING_DIRECTORY / BUILD_WORKSPACE_DIRECTORY, then relative to the inferred workspace root from argv0.
• Updates usage/docs/error output to describe the new resolution behavior.
• Adds unit tests covering Bazel env-dir resolution for literal relative paths and binary-relative fallback.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
library/tests/args.hpp	Updates documentation for -f input-file resolution order.
library/tests/args.cpp	Implements Bazel env-dir + binary-relative fallback for literal relative -f paths; updates help/error text.
library/tests/args_test.cpp	Adds tests verifying literal relative path resolution via Bazel env dirs and argv0 fallback.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tameware]

Addressed both Copilot review notes:

• Clarified resolve_dtest_input_file docs: list shorthand is for numeric ids; absolute paths only try the literal path.
• Reworded the missing-file hint so it no longer concatenates a path arg into hands/list<path>.txt.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

library/tests/args.cpp:333

• Absolute -f values that don't exist still fall through to the list-shorthand/env/binary-relative search (e.g., potentially resolving to an unrelated hands/list*.txt file). This contradicts the updated contract in args.hpp (“Absolute paths only attempt the literal path as given.”) and can produce surprising results.

    return cwd_candidate;

[tameware]

Addressed the follow-up Copilot note on absolute -f values: a missing absolute path now returns empty immediately instead of falling through to hands/list{arg}.txt lookup. Added ResolveAbsoluteMissingDoesNotUseListShorthand.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

library/tests/args.hpp:41

• The header comment says the final fallback is "relative to the directory of argv0", but the implementation actually infers the workspace root by climbing four parents from argv0 (bazel-bin/.../dtest) and then resolves relative paths from that root. This is a documentation mismatch that could confuse callers.

/// `bazel run`); then those same two forms relative to the directory of
/// `argv0` (the usual `bazel-bin/library/tests/dtest` layout). Absolute
/// paths only attempt the literal path as given.

library/tests/args_test.cpp:402

• This test assumes the OS path /no_such_dtest_abs does not exist. If it happens to exist on a developer machine or CI image, the test will fail even though the resolver logic is correct. Consider choosing a missing absolute path dynamically and creating the corresponding "trap" file based on that chosen path.

  EXPECT_TRUE(
    resolve_dtest_input_file("/no_such_dtest_abs", binary_path_).empty());

[tameware]

Addressed the latest Copilot notes:

• Docs/usage now say the argv0 fallback uses the workspace root inferred by climbing four parents from the binary (not “relative to the directory of argv0”).
• ResolveAbsoluteMissingDoesNotUseListShorthand now chooses a unique missing absolute path and builds the list-shorthand trap from that path.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

library/tests/args_test.cpp:272

• The comment references -f hands/list1.txt, but this test resolves hands/list42.txt. This is misleading when debugging bazel path resolution behavior; update the comment to match the test input.

  // bazelisk run //library/tests:dtest -- -f hands/list1.txt must find the
  // path relative to the invoke-time shell cwd, not the runfiles tree.

[tameware]

Addressed the latest Copilot note: the ResolveLiteralRelativeUsesBazelWorkingDirectory comment now refers to hands/list42.txt, matching the path the test resolves.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

library/tests/args.cpp:399

• Similarly for the binary-relative fallback: root/arg is tried before root/hands/list{arg}.txt. This can cause numeric -f 42 to resolve to a workspace-root file named 42 instead of the documented hands/list42.txt if both exist. To keep numeric shorthand semantics stable, prefer the list-shorthand candidate under the inferred workspace root before the literal candidate.

  const string bin_literal =
    normalize_logical_path((root / arg).string());
  if (path_exists(bin_literal))
    return bin_literal;

library/tests/args.cpp:389

• resolve_dtest_input_file() now checks BUILD_WORKING_DIRECTORY/arg and BUILD_WORKSPACE_DIRECTORY/arg before the numeric shorthand hands/list{arg}.txt under those env dirs. This changes numeric -f 42 resolution if a file named 42 exists in the workspace (contradicting the PR description that numeric shorthand behavior is unchanged). Consider trying the list-shorthand candidate under the Bazel env dirs before the literal arg candidate.

This issue also appears on line 395 of the same file.

  if (const string found =
        from_env_dir("BUILD_WORKING_DIRECTORY", arg); !found.empty())
  {
    return found;
  }

[tameware]

Addressed the latest Copilot notes on resolution order:

• Under BUILD_WORKING_DIRECTORY / BUILD_WORKSPACE_DIRECTORY and the argv0 workspace root, list shorthand (hands/listN.txt) is tried before a bare relative name.
• Added ResolveNumericPrefersListOverLiteralUnderBazelWorking and ResolveNumericPrefersListOverLiteralRelativeToBinary.
• Updated the resolve_dtest_input_file docs to match.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

library/tests/args.cpp:345

• from_env_dir() joins dir / rel even when rel has a root-name (e.g. Windows drive-relative C:foo). In that case std::filesystem::operator/ can discard dir, so BUILD_* / argv0-root fallback can be bypassed unexpectedly. Consider rejecting such inputs (treat them as non-relative) so the join behaves consistently.

    const char* dir = std::getenv(env_name);
    if (dir == nullptr || dir[0] == '\0')
      return string();
    const string candidate =
      normalize_logical_path((fs::path(dir) / rel).string());

library/tests/args.cpp:473

• The “Also tried …” message is misleading for absolute -f arguments: resolve_dtest_input_file() returns early for missing absolute paths and does not try BUILD_* or argv0-root fallbacks. Consider wording this as conditional (only for relative paths).

        cout << "Also tried that path under the current directory, "
          "BUILD_WORKING_DIRECTORY, BUILD_WORKSPACE_DIRECTORY, "
          "and under the workspace root inferred from the dtest binary; "
          "for numeric -f N, also hands/listN.txt\n";

[tameware]

Addressed the latest Copilot notes:

• Drive-relative -f values (Windows C:foo) are no longer joined under BUILD_* / argv0; only the literal path is tried. Added ResolveDriveRelativeMissingDoesNotJoinUnderBazelDirs (Windows).
• The missing-file “Also tried …” hint is only printed for portable relative paths, not absolute/drive-relative ones.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.