Do not use test launcher when not needed

docs/defs.md

@@ -550,7 +550,7 @@ Run the test with `bazel build //hello_lib:hello_lib_test`.
 | <a id="rust_test-data"></a>data |  List of files used by this rule at compile time and runtime.<br><br>If including data at compile time with include_str!() and similar, prefer <code>compile_data</code> over <code>data</code>, to prevent the data also being included in the runfiles.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
 | <a id="rust_test-deps"></a>deps |  List of other libraries to be linked to this library target.<br><br>These can be either other <code>rust_library</code> targets or <code>cc_library</code> targets if linking a native library.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
 | <a id="rust_test-edition"></a>edition |  The rust edition to use for this crate. Defaults to the edition specified in the rust_toolchain.   | String | optional | "" |
-| <a id="rust_test-env"></a>env |  Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to <code>$(execpath)</code> and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.   | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
+| <a id="rust_test-env"></a>env |  Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to <code>$(rootpath)</code>, <code>$(execpath)</code>, location, and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.<br><br>Execpath returns absolute path, and in order to be able to construct the absolute path we need to wrap the test binary in a launcher. Using a launcher comes with complications, such as more complicated debugger attachment.   | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
 | <a id="rust_test-proc_macro_deps"></a>proc_macro_deps |  List of <code>rust_library</code> targets with kind <code>proc-macro</code> used to help build this library target.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
 | <a id="rust_test-rustc_env"></a>rustc_env |  Dictionary of additional <code>"key": "value"</code> environment variables to set for rustc.<br><br>rust_test()/rust_binary() rules can use $(rootpath //package:target) to pass in the location of a generated file or external tool. Cargo build scripts that wish to expand locations should use cargo_build_script()'s build_script_env argument instead, as build scripts are run in a different environment - see cargo_build_script()'s documentation for more.   | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
 | <a id="rust_test-rustc_env_files"></a>rustc_env_files |  Files containing additional environment variables to set for rustc.<br><br>These files should  contain a single variable per line, of format <code>NAME=value</code>, and newlines may be included in a value by ending a line with a trailing back-slash (<code>\</code>).<br><br>The order that these files will be processed is unspecified, so multiple definitions of a particular variable are discouraged.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |

docs/flatten.md

@@ -1094,7 +1094,7 @@ Run the test with `bazel build //hello_lib:hello_lib_test`.
 | <a id="rust_test-data"></a>data |  List of files used by this rule at compile time and runtime.<br><br>If including data at compile time with include_str!() and similar, prefer <code>compile_data</code> over <code>data</code>, to prevent the data also being included in the runfiles.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
 | <a id="rust_test-deps"></a>deps |  List of other libraries to be linked to this library target.<br><br>These can be either other <code>rust_library</code> targets or <code>cc_library</code> targets if linking a native library.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
 | <a id="rust_test-edition"></a>edition |  The rust edition to use for this crate. Defaults to the edition specified in the rust_toolchain.   | String | optional | "" |
-| <a id="rust_test-env"></a>env |  Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to <code>$(execpath)</code> and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.   | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
+| <a id="rust_test-env"></a>env |  Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to <code>$(rootpath)</code>, <code>$(execpath)</code>, location, and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.<br><br>Execpath returns absolute path, and in order to be able to construct the absolute path we need to wrap the test binary in a launcher. Using a launcher comes with complications, such as more complicated debugger attachment.   | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
 | <a id="rust_test-proc_macro_deps"></a>proc_macro_deps |  List of <code>rust_library</code> targets with kind <code>proc-macro</code> used to help build this library target.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
 | <a id="rust_test-rustc_env"></a>rustc_env |  Dictionary of additional <code>"key": "value"</code> environment variables to set for rustc.<br><br>rust_test()/rust_binary() rules can use $(rootpath //package:target) to pass in the location of a generated file or external tool. Cargo build scripts that wish to expand locations should use cargo_build_script()'s build_script_env argument instead, as build scripts are run in a different environment - see cargo_build_script()'s documentation for more.   | <a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a> | optional | {} |
 | <a id="rust_test-rustc_env_files"></a>rustc_env_files |  Files containing additional environment variables to set for rustc.<br><br>These files should  contain a single variable per line, of format <code>NAME=value</code>, and newlines may be included in a value by ending a line with a trailing back-slash (<code>\</code>).<br><br>The order that these files will be processed is unspecified, so multiple definitions of a particular variable are discouraged.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |

rust/private/rust.bzl

@@ -306,13 +306,14 @@ def _rust_binary_impl(ctx):
         ),
     )
 
-def _create_test_launcher(ctx, toolchain, output, providers):
+def _create_test_launcher(ctx, toolchain, output, env, providers):
     """Create a process wrapper to ensure runtime environment variables are defined for the test binary
 
     Args:
         ctx (ctx): The rule's context object
         toolchain (rust_toolchain): The current rust toolchain
         output (File): The output File that will be produced, depends on crate type.
+        env (dict): Dict of environment variables
         providers (list): Providers from a rust compile action. See `rustc_compile_action`
 
     Returns:
@@ -337,20 +338,12 @@ def _create_test_launcher(ctx, toolchain, output, providers):
         is_executable = True,
     )
 
-    # Get data attribute
-    data = getattr(ctx.attr, "data", [])
-
     # Expand the environment variables and write them to a file
     environ_file = ctx.actions.declare_file(launcher_filename + ".launchfiles/env")
-    environ = expand_dict_value_locations(
-        ctx,
-        getattr(ctx.attr, "env", {}),
-        data,
-    )
 
     # Convert the environment variables into a list to be written into a file.
     environ_list = []
-    for key, value in sorted(environ.items()):
+    for key, value in sorted(env.items()):
         environ_list.extend([key, value])
 
     ctx.actions.write(
@@ -454,8 +447,21 @@ def _rust_test_common(ctx, toolchain, output):
         crate_info = crate_info,
         rust_flags = ["--test"] if ctx.attr.use_libtest_harness else ["--cfg", "test"],
     )
+    data = getattr(ctx.attr, "data", [])
+
+    env = expand_dict_value_locations(
+        ctx,
+        getattr(ctx.attr, "env", {}),
+        data,
+    )
+    providers.append(testing.TestEnvironment(env))
 
-    return _create_test_launcher(ctx, toolchain, output, providers)
+    if any(["{pwd}" in v for v in env.values()]):
+        # Some of the environment variables require expanding {pwd} placeholder at runtime,
+        # we need a launcher for that.
+        return _create_test_launcher(ctx, toolchain, output, env, providers)
+    else:
+        return providers
 
 def _rust_test_impl(ctx):
     """The implementation of the `rust_test` rule
@@ -630,8 +636,12 @@ _rust_test_attrs = {
         mandatory = False,
         doc = dedent("""\
             Specifies additional environment variables to set when the test is executed by bazel test.
-            Values are subject to `$(execpath)` and
+            Values are subject to `$(rootpath)`, `$(execpath)`, location, and
             ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.
+
+            Execpath returns absolute path, and in order to be able to construct the absolute path we
+            need to wrap the test binary in a launcher. Using a launcher comes with complications, such as
+            more complicated debugger attachment.
         """),
     ),
     "use_libtest_harness": attr.bool(

test/test_env/BUILD.bazel

@@ -26,6 +26,5 @@ rust_test(
     env = {
         "FERRIS_SAYS": "Hello fellow Rustaceans!",
         "HELLO_WORLD_BIN_ROOTPATH": "$(rootpath :hello-world)",
-        "HELLO_WORLD_SRC_EXECPATH": "$(execpath :hello_world_main)",
     },
 )

test/test_env/tests/run.rs

@@ -26,10 +26,4 @@ fn run() {
     );
     assert!(!hello_world_bin.is_absolute());
     assert!(hello_world_bin.exists());
-
-    // Ensure `execpath` expanded variables map to real files and have absolute paths
-    let hello_world_src =
-        std::path::PathBuf::from(std::env::var("HELLO_WORLD_SRC_EXECPATH").unwrap());
-    assert!(hello_world_src.is_absolute());
-    assert!(hello_world_src.exists());
 }

test/test_env_launcher/BUILD.bazel

@@ -0,0 +1,31 @@
+load(
+    "//rust:rust.bzl",
+    "rust_binary",
+    "rust_test",
+)
+
+rust_binary(
+    name = "hello-world",
+    srcs = ["src/main.rs"],
+    edition = "2018",
+)
+
+filegroup(
+    name = "hello_world_main",
+    srcs = ["src/main.rs"],
+)
+
+rust_test(
+    name = "test",
+    srcs = ["tests/run.rs"],
+    data = [
+        ":hello-world",
+        ":hello_world_main",
+    ],
+    edition = "2018",
+    env = {
+        "FERRIS_SAYS": "Hello fellow Rustaceans!",
+        "HELLO_WORLD_BIN_ROOTPATH": "$(rootpath :hello-world)",
+        "HELLO_WORLD_SRC_EXECPATH": "$(execpath :hello_world_main)",
+    },
+)

test/test_env_launcher/src/main.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("Hello world");
+}

test/test_env_launcher/tests/run.rs

@@ -0,0 +1,35 @@
+#[test]
+fn run() {
+    let path = env!("CARGO_BIN_EXE_hello-world");
+    let output = std::process::Command::new(path)
+        .output()
+        .expect("Failed to run process");
+    assert_eq!(&b"Hello world\n"[..], output.stdout.as_slice());
+
+    // Test the `env` attribute of `rust_test` at run time
+    assert_eq!(
+        std::env::var("FERRIS_SAYS").unwrap(),
+        "Hello fellow Rustaceans!"
+    );
+
+    // Test the behavior of `rootpath` and that a binary can be found relative to current_dir
+    let hello_world_bin =
+        std::path::PathBuf::from(std::env::var_os("HELLO_WORLD_BIN_ROOTPATH").unwrap());
+
+    assert_eq!(
+        hello_world_bin.as_path(),
+        std::path::Path::new(if std::env::consts::OS == "windows" {
+            "test/test_env_launcher/hello-world.exe"
+        } else {
+            "test/test_env_launcher/hello-world"
+        })
+    );
+    assert!(!hello_world_bin.is_absolute());
+    assert!(hello_world_bin.exists());
+
+    // Ensure `execpath` expanded variables map to real files and have absolute paths
+    let hello_world_src =
+        std::path::PathBuf::from(std::env::var("HELLO_WORLD_SRC_EXECPATH").unwrap());
+    assert!(hello_world_src.is_absolute());
+    assert!(hello_world_src.exists());
+}