Clarify expected behavior for FIXME in build/fbcode_builder/getdeps/builder.py

[Virendra2406087]

Summary

While reading build/fbcode_builder/getdeps/builder.py, I noticed the following FIXME:

# FIXME: What is this trying to accomplish?
# Should it fail on first or >=1 errors?

The code currently iterates over runs and invokes _run_cmd() for each entry, but I'm not sure what the intended failure handling is:

for run in runs:
    self._run_cmd(
        testpilot_args + run,
        cwd=self.build_opts.fbcode_builder_dir,
        env=env,
        use_cmd_prefix=use_cmd_prefix,
    )

I'm interested in contributing to this area and wanted to clarify the intended behavior before preparing a patch.

Should the implementation:

1. Stop immediately when the first _run_cmd() invocation returns a non-zero exit code, or
2. Execute all configured runs and report failure if one or more invocations return a non-zero exit code?

I'd be happy to work on a PR once the expected behavior is confirmed. Thanks!

[hiSandog]

Are the entries in runs independent test shards/configurations, or sequential steps where later runs depend on earlier output? If they are independent, executing all and returning a summary of failed commands gives much better CI diagnostics; dependent steps should fail fast to avoid misleading follow-on errors. Whichever behavior is intended, preserving the first non-zero code plus listing every failed invocation would make the FIXME's contract testable.

[Virendra2406087]

Thanks for the suggestion!

From what I can tell, runs is populated based on the selected schedule_type. For example, when schedule_type == "testwarden", multiple entries are added to runs, and each entry is then passed to _run_cmd() in a loop.

I wasn't sure whether these entries are intended to represent independent test configurations or sequential steps, which is why I opened this issue. I'll wait for clarification from the maintainers on the intended behavior before preparing a patch.