From 021a77ce5a7734c0056cf0aca9ffea4ec8759ca6 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Tue, 5 Nov 2024 09:55:22 +0000 Subject: [PATCH 1/8] refactor(e2e): use test suite This will allow running all Docker tests at once. --- e2e/docker/BUILD.bazel | 70 +++++++++++++++-------------- e2e/docker/{world.log => hello.txt} | 0 2 files changed, 37 insertions(+), 33 deletions(-) rename e2e/docker/{world.log => hello.txt} (100%) diff --git a/e2e/docker/BUILD.bazel b/e2e/docker/BUILD.bazel index 4c1c2cd0..4509a967 100644 --- a/e2e/docker/BUILD.bazel +++ b/e2e/docker/BUILD.bazel @@ -5,6 +5,11 @@ load("@rules_labgrid//labgrid/run:defs.bzl", "labgrid_run") load("@rules_labgrid//labgrid/state:defs.bzl", "labgrid_state") load("@rules_python//python:defs.bzl", "py_binary") +TAGS = [ + "manual", + "requires-docker", +] + # A constraint to register the toolchain to constraint_setting(name = "device") @@ -49,68 +54,67 @@ py_binary( deps = ["@pip//docker"], ) -# A simplistic implementation of cat which dependends on runfiles py_binary( - name = "cat", + name = "cat-with-runfiles", srcs = ["cat.py"], + main = "cat.py", ) labgrid_run( - name = "echo", - srcs = [":cat"], - outs = ["stdout.log"], - cmd = "$(locations :cat) /etc/os-release > $@", + name = "cat", + srcs = [":cat-with-runfiles"], + outs = ["run-on-device-with-runfiles.actual"], + cmd = "$(locations :cat-with-runfiles) /etc/os-release > $@", platform = ":platform", - tags = [ - "manual", - "requires-docker", - ], ) labgrid_genrule( - name = "copy", + name = "cp", srcs = ["@ape//ape:cp"], - outs = ["output.txt"], - cmd = "$(location @rules_labgrid//labgrid/run) --get output.txt:$(location output.txt) $(location @ape//ape:cp) /etc/os-release output.txt", + outs = ["run-on-device-and-transfer-file-back.actual"], + cmd = "$(location @rules_labgrid//labgrid/run) --get output.txt:$@ $(location @ape//ape:cp) /etc/os-release output.txt", platform = ":platform", tools = ["@rules_labgrid//labgrid/run"], ) [ - # Check that the output of `/proc/version` from within the container is what we expect diff_file_test( - name = "test-{textfile}".format(textfile = textfile), + name = name, size = "small", a = ":version.txt", - b = textfile, - tags = [ - "manual", - "requires-docker", - ], + b = "{}.actual".format(name), + tags = TAGS, + ) + for name in ( + "run-on-device-with-runfiles", + "run-on-device-and-transfer-file-back", ) - for textfile in ("output.txt", "stdout.log") ] labgrid_genrule( - name = "hello_world", + name = "printenv", srcs = ["@ape//ape:printenv"], + outs = ["run-on-device-with-env.actual"], cmd = "$(location @rules_labgrid//labgrid/run) --env HELLO=world -- $(location @ape//ape:printenv) HELLO > $@", - outs = ["hello_world.log"], platform = ":platform", tools = ["@rules_labgrid//labgrid/run"], - tags = [ - "manual", - "requires-docker", - ], ) diff_file_test( - name = "hello_world_test", + name = "run-on-device-with-env", size = "small", - a = ":world.log", - b = ":hello_world.log", - tags = [ - "manual", - "requires-docker", + a = ":hello.txt", + b = ":run-on-device-with-env.actual", + tags = TAGS, +) + +# FIXME: This currently fails with a concurrency error +test_suite( + name = "test", + tags = ["manual"], + tests = [ + ":run-on-device-and-transfer-file-back", + ":run-on-device-with-env", + ":run-on-device-with-runfiles", ], ) diff --git a/e2e/docker/world.log b/e2e/docker/hello.txt similarity index 100% rename from e2e/docker/world.log rename to e2e/docker/hello.txt -- GitLab From b55a08480a467964d0ddf1e5202eff4f113f1552 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Tue, 5 Nov 2024 09:56:53 +0000 Subject: [PATCH 2/8] feat(executor): allow runfile path for program --- labgrid/executor/executor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/labgrid/executor/executor.py b/labgrid/executor/executor.py index 184b1285..90a3c241 100644 --- a/labgrid/executor/executor.py +++ b/labgrid/executor/executor.py @@ -56,6 +56,7 @@ def arguments(prsr: ArgumentParser) -> None: "program", metavar="PROG", help="The binary to run within the LabGrid environment.", + type=resolve, ) prsr.add_argument( "arguments", -- GitLab From df4e0b5667e1b72cd8bb66bebb627fbbeeb46462 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Tue, 5 Nov 2024 09:57:42 +0000 Subject: [PATCH 3/8] feat(run): allow runfile path for program --- labgrid/run/run.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 52e306ae..849dbf9b 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -48,6 +48,13 @@ class Tools: mkdir: Path = runfile("ape/ape/assimilate/mkdir.ape/mkdir") env: Path = runfile("ape/ape/assimilate/env.ape/env") +def resolve(value: str) -> Path: + try: + return runfile(value) + except RunfileNotFoundError: + return Path(value) + + @dataclass(frozen=True) class State: desired: str @@ -120,7 +127,7 @@ def env(arg: str) -> str: def arguments(prsr: ArgumentParser) -> None: prsr.add_argument( - "program", metavar="PROG", help="The program to run on the device.", type=Path + "program", metavar="PROG", help="The program to run on the device.", type=resolve ) prsr.add_argument( "arguments", metavar="ARG", nargs=REMAINDER, help="Command to run over SSH." -- GitLab From f3c774418a77423d44467300c65633bb6b80369a Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Tue, 5 Nov 2024 10:05:25 +0000 Subject: [PATCH 4/8] feat(run): support `RUNFILES_DIR` Currently, we assume the runfiles for the program will follow the convention of being a folder under the same path with the `.runfiles` suffix. However, in certain situations that's not the case, so we need to use the `RUNFILES_DIR` as a fallback. --- labgrid/run/run.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 849dbf9b..f0d30ba6 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -214,8 +214,11 @@ def run( env: Iterator[str], tools: Tools = Tools() ) -> int: + runfiles_dir = None # Allow any Bazel binaries used in the configuration to work try: + # FIXME: Convert into context manager + runfiles_dir = environ["RUNFILES_DIR"] del environ["RUNFILES_DIR"] except KeyError: pass @@ -238,6 +241,8 @@ def run( dest_runfiles = remote.with_suffix(".runfiles") if src_runfiles.exists(): transfer.put(f"{src_runfiles}", f"{dest_runfiles}") + elif runfiles_dir is not None: + transfer.put(f"{runfiles_dir}", f"{dest_runfiles}") # Transfer 'env' binary over to the device transfer.put(f"{tools.env}", f"{temp / tools.env.name}") -- GitLab From 26c59e8059410620a2e949ef9d8a1ef75123bda2 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Wed, 6 Nov 2024 14:53:21 +0000 Subject: [PATCH 5/8] fix(run): support quoted `--get` value `ctx.actions.args()` will add quotes to values that contain characters that need escaping (such as `:`). --- labgrid/run/run.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index f0d30ba6..5bad3405 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -181,7 +181,8 @@ def arguments(prsr: ArgumentParser) -> None: def get(value: str) -> Get: - remote, found, local = value.partition(":") + # FIXME: Extract `BazelArgumentParser` to handle quoting + remote, found, local = value.removeprefix("'").removesuffix("'").partition(":") if found: return Get(PurePath(remote), PurePath(local)) return Get(PurePath(remote), PurePath(remote)) -- GitLab From dc6be0337851cfd6e10209f0e4b91e2fcd8f5f9d Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Thu, 7 Nov 2024 10:12:12 +0000 Subject: [PATCH 6/8] feat(run): allow optional remote path in `--get` --- labgrid/run/run.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 5bad3405..f30ebb43 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -26,6 +26,7 @@ from python.runfiles import Runfiles from bazel.labgrid.strategy import transition from bazel.labgrid.util.target import TemporaryDirectory from labgrid import Environment +from labgrid.driver.exception import ExecutionError T = TypeVar("T") @@ -172,9 +173,9 @@ def arguments(prsr: ArgumentParser) -> None: ) prsr.add_argument( "--get", - help="Transfer files from target at end of execution. Relative paths are resolved to the execution root on both the local and remote. `LOCAL` can be omitted, which will use the same path for remote and local.", + help="Transfer files from target at end of execution. Relative paths are resolved to the execution root on both the local and remote. `REMOTE` can have a trailing `?` to indicate it's optional. `LOCAL` can be omitted, which will use the same path for remote and local.", type=get, - metavar="REMOTE[:LOCAL]", + metavar="REMOTE[?][:LOCAL]", action="append", default=[], ) @@ -183,20 +184,25 @@ def arguments(prsr: ArgumentParser) -> None: def get(value: str) -> Get: # FIXME: Extract `BazelArgumentParser` to handle quoting remote, found, local = value.removeprefix("'").removesuffix("'").partition(":") - if found: - return Get(PurePath(remote), PurePath(local)) - return Get(PurePath(remote), PurePath(remote)) + optional = remote.endswith("?") + if optional: + remote = remote.removesuffix("?") + if not found: + local = remote + return Get(PurePath(remote), PurePath(local), optional) @dataclass(frozen=True) class Get: remote: PurePath local: PurePath + optional: bool def resolve(self, remote: PurePath) -> Get: return Get( remote=self.join(remote, self.remote), local=self.join(environ.get("BAZEL_GEN_DIR", Path.cwd()), self.local), + optional=self.optional, ) @staticmethod @@ -259,11 +265,16 @@ def run( for line in err: stderr.write(f"{line}{linesep}") - if code == 0: # copy the files back from the target - + # Copy the files back from the target + if code == 0: for unresolved in get: resolved = unresolved.resolve(temp) - transfer.get(resolved.remote, resolved.local) + # FIXME: Check if file exists with `@ape//ape:test` instead of assuming any error is due to a missing file + try: + transfer.get(resolved.remote, resolved.local) + except ExecutionError as e: + if not resolved.optional: + raise e return code -- GitLab From 4dd3b2572f109f9f08246d580d4bc13fba8ccc1f Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Thu, 7 Nov 2024 11:23:26 +0000 Subject: [PATCH 7/8] feat(run): support env vars in local path for `--get` --- labgrid/run/run.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index f30ebb43..d75e1b04 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -7,6 +7,7 @@ from dataclasses import dataclass, replace from os import environ, linesep, getenv from pathlib import Path, PurePath from shlex import join +from string import Template from subprocess import CalledProcessError from sys import argv, stderr, stdout from typing import ( @@ -173,7 +174,7 @@ def arguments(prsr: ArgumentParser) -> None: ) prsr.add_argument( "--get", - help="Transfer files from target at end of execution. Relative paths are resolved to the execution root on both the local and remote. `REMOTE` can have a trailing `?` to indicate it's optional. `LOCAL` can be omitted, which will use the same path for remote and local.", + help="Transfer files from target at end of execution. Relative paths are resolved to the execution root on both the local and remote. `REMOTE` can have a trailing `?` to indicate it's optional. `LOCAL` performs environment variable substitution. `LOCAL` can be omitted, which will use the same path for remote and local.", type=get, metavar="REMOTE[?][:LOCAL]", action="append", @@ -187,7 +188,9 @@ def get(value: str) -> Get: optional = remote.endswith("?") if optional: remote = remote.removesuffix("?") - if not found: + if found: + local = Template(local).substitute(environ) + else: local = remote return Get(PurePath(remote), PurePath(local), optional) -- GitLab From 26f809f393493c41ed88879c97ec84ca2b5447b9 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Tue, 5 Nov 2024 10:10:49 +0000 Subject: [PATCH 8/8] feat(test): allow running test executables on the device --- e2e/docker/BUILD.bazel | 17 +++++++ e2e/docker/test_os_release.py | 18 +++++++ labgrid/test/BUILD.bazel | 0 labgrid/test/defs.bzl | 5 ++ labgrid/test/rule.bzl | 88 +++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+) create mode 100644 e2e/docker/test_os_release.py create mode 100644 labgrid/test/BUILD.bazel create mode 100644 labgrid/test/defs.bzl create mode 100644 labgrid/test/rule.bzl diff --git a/e2e/docker/BUILD.bazel b/e2e/docker/BUILD.bazel index 4509a967..25070692 100644 --- a/e2e/docker/BUILD.bazel +++ b/e2e/docker/BUILD.bazel @@ -3,6 +3,7 @@ load("@rules_labgrid//labgrid/config/toolchain:defs.bzl", "labgrid_config_toolch load("@rules_labgrid//labgrid/genrule:defs.bzl", "labgrid_genrule") load("@rules_labgrid//labgrid/run:defs.bzl", "labgrid_run") load("@rules_labgrid//labgrid/state:defs.bzl", "labgrid_state") +load("@rules_labgrid//labgrid/test:defs.bzl", "labgrid_test") load("@rules_python//python:defs.bzl", "py_binary") TAGS = [ @@ -108,6 +109,21 @@ diff_file_test( tags = TAGS, ) +py_binary( + name = "inner", + srcs = ["test_os_release.py"], + data = [":version.txt"], + main = "test_os_release.py", + deps = ["@rules_python//python/runfiles"], +) + +labgrid_test( + name = "test-on-device", + src = ":inner", + platform = ":platform", + tags = TAGS, +) + # FIXME: This currently fails with a concurrency error test_suite( name = "test", @@ -116,5 +132,6 @@ test_suite( ":run-on-device-and-transfer-file-back", ":run-on-device-with-env", ":run-on-device-with-runfiles", + ":test-on-device", ], ) diff --git a/e2e/docker/test_os_release.py b/e2e/docker/test_os_release.py new file mode 100644 index 00000000..8928885a --- /dev/null +++ b/e2e/docker/test_os_release.py @@ -0,0 +1,18 @@ +import unittest +from pathlib import Path + +from python.runfiles import Runfiles + +def _cat(path): + return Path(path).read_text() + +def _runfile(path): + r = Runfiles.Create() + return Path(r.Rlocation(path)).read_text() + +class OSReleaseTestCase(unittest.TestCase): + def test(self): + self.assertEqual(_cat("/etc/os-release"), _runfile("_main/docker/version.txt")) + +if __name__ == "__main__": + unittest.main() diff --git a/labgrid/test/BUILD.bazel b/labgrid/test/BUILD.bazel new file mode 100644 index 00000000..e69de29b diff --git a/labgrid/test/defs.bzl b/labgrid/test/defs.bzl new file mode 100644 index 00000000..6ad63a44 --- /dev/null +++ b/labgrid/test/defs.bzl @@ -0,0 +1,5 @@ +load(":rule.bzl", _test = "test") + +visibility("public") + +labgrid_test = _test diff --git a/labgrid/test/rule.bzl b/labgrid/test/rule.bzl new file mode 100644 index 00000000..46f97f65 --- /dev/null +++ b/labgrid/test/rule.bzl @@ -0,0 +1,88 @@ +load("//labgrid/cfg:store.bzl", _cfg = "store") + +visibility("//...") + +DOC = """Run a test executable on the device.""" + +ATTRS = { + "src": attr.label( + doc = "The test executable to run on the Labgrid device", + executable = True, + cfg = "target", + ), + "platform": attr.label( + doc = "Platform to transition to.", + providers = [platform_common.PlatformInfo], + ), + "_executor": attr.label( + doc = "The LabGrid environment executor", + default = "//labgrid/executor", + executable = True, + cfg = "exec", + ), + "_run": attr.label( + doc = "The LabGrid run", + default = "//labgrid/run", + executable = True, + cfg = "exec", + ), +} + +def _runfile(label, file): + path = file.short_path + if path.startswith("../"): + return path.removeprefix("../") + return "{}/{}".format(label.workspace_name or "_main", path) + +def _env(pair): + key, value = pair + return ["--env", "{}={}".format(key, value)] + +def _get(pair): + key, value = pair + return ["--get", "{}:{}".format(key, value)] + +def implementation(ctx): + env = { + "XML_OUTPUT_FILE": "results.xml", + "BAZEL_TEST": "1", + } + + get = { + "results.xml?": "${XML_OUTPUT_FILE}", + } + + arguments = ctx.actions.declare_file("{}.args".format(ctx.label.name)) + args = ctx.actions.args() + args.add("--") + args.add(_runfile(ctx.attr._run.label, ctx.executable._run)) + args.add_all(env.items(), map_each = _env) + args.add_all(get.items(), map_each = _get) + args.add("--") + args.add(_runfile(ctx.attr.src.label, ctx.executable.src)) + ctx.actions.write(output = arguments, content = args) + + executable = ctx.actions.declare_file(ctx.label.name) + ctx.actions.symlink( + output = executable, + target_file = ctx.executable._executor, + is_executable = True, + ) + + files = depset([executable, arguments]) + root_symlinks = {"extra.args": arguments} + runfiles = ctx.runfiles([ctx.executable.src], root_symlinks = root_symlinks) + runfiles = runfiles.merge(ctx.attr._executor.default_runfiles) + runfiles = runfiles.merge(ctx.attr._run.default_runfiles) + runfiles = runfiles.merge(ctx.attr.src.default_runfiles) + return DefaultInfo(executable = executable, files = files, runfiles = runfiles) + +labgrid_test = rule( + doc = DOC, + attrs = ATTRS, + implementation = implementation, + cfg = _cfg, + test = True, +) + +test = labgrid_test -- GitLab