Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transition to exec configuration for bazel test --run_under=... --platforms=... #22624

Closed
tpudlik opened this issue Jun 3, 2024 · 9 comments
Closed
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug

Comments

@tpudlik
Copy link
Contributor

tpudlik commented Jun 3, 2024

Description of the bug:

bazel test --run_under=:test_runner --platforms=:device is a natural way to run on-device tests with Bazel: you point --run_under to your test runner (which flashes tests to the hardware), and --platforms to the target device platform.

But it's awkward in practice because the target that --run_under points to is built for the target platform, even though it will execute on the exec platform. So you need to wrap your runner into a platform_data:

platform_data(
  name = "run_under_wrapper",
  target = ":test_runner",
  platform = "@local_config_platform//:host",
)

@katre @gregestren

Which category does this issue belong to?

Configurability

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

No response

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No.

Have you found anything relevant by searching the web?

This is similar to the case of bazel run --run_under --platforms, discussed here.

Any other information, logs, or outputs that you want to share?

No response

@github-actions github-actions bot added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Jun 3, 2024
@katre
Copy link
Member

katre commented Jun 4, 2024

It seems like the right answer here is that, for the command bazel test --run_under=:runner --platforms=:platform :test, we should have the following:

  1. :runner should be built for the test execution platform (whatever that is), probably the host platform but maybe some form of remote exec platform.
  2. :test should be built for the platform specified by --platforms
  3. If these aren't compatible, it's up to the runner to report the error in a useful fashion.

Is that a reasonable summary?

@katre
Copy link
Member

katre commented Jun 4, 2024

I suspect that bazel run will also need similar handling. A quick glance at the code makes it seem like these are handled very differently.

@tpudlik
Copy link
Contributor Author

tpudlik commented Jun 4, 2024

Yes, that summary sounds right to me.

@tpudlik
Copy link
Contributor Author

tpudlik commented Jun 4, 2024

It turns out that the platform_data workaround I mentioned is somewhat inconvenient because of bazelbuild/rules_platform#11: if the binary target you wanted to invoke with --run_under had an args attribute, that's lost when the binary is wrapped by platform_data. In fact it's not clear (at least to me) how to implement a rule that forwards the args through a platform transition.

@gregestren
Copy link
Contributor

What happens if :runner is only target-compatible with a Mac platform?

Should that change the whole test to run on a Mac execution platform?

Stated differently, should exec platform resolution be based on :runner's compatibility settings?

@tpudlik
Copy link
Contributor Author

tpudlik commented Jul 16, 2024

I'm not sure how this should interact with exec platform resolution: I spend all my time in the simple world where the exec and host platforms are one and the same! If my host platform is Linux and the :runner is only target-compatible with a Mac platform, I'd just expect an error stating that.

@gregestren gregestren self-assigned this Jul 17, 2024
@gregestren gregestren added P1 I'll work on this now. (Assignee required) and removed untriaged labels Jul 17, 2024
@gregestren
Copy link
Contributor

@tpudlik for $ bazel test --run_under=:test_runner --platforms=:device :my_test, doesn't Bazel still expect that my_test is a test rule? Does this imply two test runners? test_runner does the flashing setup and my_test runs the tests on the actual device?

I can also see scenarios where :my_test should use the exec config. Bazel ultimately runs $ ./test_runner my_test on the test machine. Obviously it's up to both binaries how they interpret that. I wonder if we need some API expansion to support different intentions.

@gregestren
Copy link
Contributor

Closed this specific issue, which is bazel test with --run_under. But there's also bazel run and default test runners. See #21805 (reply in thread) for wider context and similar fixes we might do for those.

I looked into (but didn't fix) doing the same for $ bazel run --run_under. Conceptually it's straightforward. But the bazel run logic is completely different than bazel test. We'd have to implement an exec transition on a top-level build target request. I think we can technically already do that with

public StarlarkAttributeTransitionProvider getStarlarkExecTransitionForTesting(
But that'shacky and I don't think we should have more ad hoc logic in SkyframeExecutor for transitions. We might be able to copy

or even add some implicit exec dependency on top-level targets just to get access to the exec configuration.

@tpudlik
Copy link
Contributor Author

tpudlik commented Jul 30, 2024

Yes, Bazel does expect that my_test is a test rule. It needs to be implemented using some unit-testing framework, which is up to the user to choose. If you choose one that's compatible with embedded environments (like pw_unit_test), the unit test binary will be executable on the embedded device, and will run the unit-tests in the sources of my_test. I suppose that makes it a "runner"!

I can also see scenarios where :my_test should use the exec config. Bazel ultimately runs $ ./test_runner my_test on the test machine. Obviously it's up to both binaries how they interpret that. I wonder if we need some API expansion to support different intentions.

If you want :my_test to use the exec config, you can just omit the --platforms argument and do bazel --run_under=:test_runner :my_test, right? Ah I guess maybe not quite, in that case :my_test is built for the host rather than execution platform. I don't have a good intuition for the cases where these two platforms are different---I suppose it mostly arises in the context of remote execution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug
Projects
None yet
Development

No branches or pull requests

6 participants
@katre @tpudlik @gregestren @iancha1992 @satyanandak and others