remote_test_execution is undocumented

[benbrittain]

I went to go upgrade to buck 2023-11-01, and there is a new toolchain that seems to be mandatory. I'm not sure what to do with it in an open source context.

[thoughtpolice]

You probably just need to add remote_test_execution_toolchain to your top-level BUCK file in the toolchains// cell, just like any other:

• https://github.com/facebook/buck2/blob/735438938264a64ad2ffc2d908f1ab77cff9ae3f/prelude/toolchains/remote_test_execution.bzl
• https://github.com/facebook/buck2/blob/735438938264a64ad2ffc2d908f1ab77cff9ae3f/prelude/toolchains/demo.bzl#L57C5-L57C36

[benbrittain]

Yes, but that's not actually indicative of what to populate the values with or what they are for. This is a new toolchain that is apparently going to be mandatory for anyone using the standard prelude. Having some documentation (or even doc ="..." on the attributes) seems important.

[thoughtpolice]

Ah, sorry, I misread the original title, I just thought you wanted your build working, really. Anyway, the primary use case inside re_utils.bzl seems to just be to shim the given set of RE properties into a test rule's CommandExecutorConfig, so that the test can run on the right remote execution service. That is, whatever there's a remote_execution property on a (supported) _test rule, it takes the value of that property and then looks it up in the profiles map of the remote execution test toolchain. It then attaches a CommandExecutorConfig with those RE properties. Something like:

rust_test(
  name = "test1"
  remote_execution = "rust_re_toolchain"
)

remote_test_execution_toolchain(
  ...
  profiles = {
    "rust_re_toolchain": {
      "use_case": "rust-testing"
      "capabilities": { ... } # re properties go here
    }
  }
)

There's also a default_profile which is fairly self-explanatory. This design means you mostly need to update the remote_test_execution_toolchain call in order to change the set of RE properties, rather than changing it everywhere.

Frankly, I just read the source code for everything if I need it, at this point. It's quite readable in this case, though I don't disagree it should be documented:

• https://github.com/facebook/buck2/blob/735438938264a64ad2ffc2d908f1ab77cff9ae3f/prelude/tests/re_utils.bzl#L37C9-L42

Looking around for get_re_executor_from_props it looks like it's only supported for a few things like cxx_test, rust_test, and sh_test.

[ndmitchell]

Urk, it's rather unfortunate this is mandatory - ideally open source users wouldn't have to care at all. Given it is mandatory we should definitely document, but still worth figuring out if we can make it opt in (if you want the complexity then sure, but otherwise, not)