From f8ef767ab0057d634c60600d46451698ba25de29 Mon Sep 17 00:00:00 2001
From: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Date: Thu, 22 Aug 2024 09:54:26 -0700
Subject: [PATCH] compile_fail_utils: Verify path exists (#14827)

# Objective

It looks like running `compile_fail_utils::test` on an invalid path just
skips the test. I'm not sure why `ui_test` doesn't fail the test, but it
seems pretty easy to accidentally cause a test to be skipped (I
experienced
[this](https://github.com/bevyengine/bevy/pull/14813#discussion_r1721880973)
while doing some refactoring on `bevy_reflect`).

## Solution

Check to make sure the given path exists before continuing on with the
tests.

Alternatively, we could look into seeing why this doesn't work properly
upstream. But I figured this solution was simple enough just to
implement directly without having to worry about updating `ui_test`.

## Testing

To verify that this works as expected `cd` into
`crates/bevy_reflect/compile_fail`. Then run the following:

```
cargo test --target-dir ../../../target
```

All compile fail tests should pass. Now edit the path used in
`crates/bevy_reflect/compile_fail/tests/derive.rs`. For example:

```diff
fn main() -> compile_fail_utils::ui_test::Result<()> {
-    compile_fail_utils::test("reflect_derive", "tests/reflect_derive")
+    compile_fail_utils::test("reflect_derive", "tests/does_not_exist")
}
```

Run the tests again:

```
cargo test --target-dir ../../../target
```

Verify the test fails with an error like:

```
Error: path does not exist: "tests/does_not_exist"
```
---
 tools/compile_fail_utils/src/lib.rs | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/compile_fail_utils/src/lib.rs b/tools/compile_fail_utils/src/lib.rs
index 05559e193a..ff6383cc22 100644
--- a/tools/compile_fail_utils/src/lib.rs
+++ b/tools/compile_fail_utils/src/lib.rs
@@ -7,6 +7,7 @@ use std::{
 pub use ui_test;
 
 use ui_test::{
+    color_eyre::eyre::eyre,
     default_file_filter, default_per_file_config,
     dependencies::DependencyBuilder,
     run_tests_generic,
@@ -20,7 +21,19 @@ use ui_test::{
 /// `root_dir` is the directory your tests are contained in. Needs to be a path from crate root.
 /// This config will build dependencies and will assume that the cargo manifest is placed at the
 /// current working directory.
-fn basic_config(root_dir: impl Into<PathBuf>, args: &Args) -> Config {
+fn basic_config(root_dir: impl Into<PathBuf>, args: &Args) -> ui_test::Result<Config> {
+    let root_dir = root_dir.into();
+
+    match root_dir.try_exists() {
+        Ok(true) => { /* success */ }
+        Ok(false) => {
+            return Err(eyre!("path does not exist: {:?}", root_dir));
+        }
+        Err(error) => {
+            return Err(eyre!("failed to read path: {:?} ({:?})", root_dir, error));
+        }
+    }
+
     let mut config = Config {
         bless_command: Some(
             "`cargo test` with the BLESS environment variable set to any non empty value"
@@ -60,7 +73,7 @@ fn basic_config(root_dir: impl Into<PathBuf>, args: &Args) -> Config {
         Spanned::dummy(vec![Box::new(DependencyBuilder::default())]),
     );
 
-    config
+    Ok(config)
 }
 
 /// Runs ui tests for a single directory.
@@ -72,7 +85,7 @@ pub fn test(test_name: impl Into<String>, test_root: impl Into<PathBuf>) -> ui_t
 
 /// Run ui tests with the given config
 pub fn test_with_config(test_name: impl Into<String>, config: Config) -> ui_test::Result<()> {
-    test_with_multiple_configs(test_name, [config])
+    test_with_multiple_configs(test_name, [Ok(config)])
 }
 
 /// Runs ui tests for a multiple directories.
@@ -94,9 +107,9 @@ pub fn test_multiple(
 /// Tests for configs are run in parallel.
 pub fn test_with_multiple_configs(
     test_name: impl Into<String>,
-    configs: impl IntoIterator<Item = Config>,
+    configs: impl IntoIterator<Item = ui_test::Result<Config>>,
 ) -> ui_test::Result<()> {
-    let configs = configs.into_iter().collect();
+    let configs = configs.into_iter().collect::<ui_test::Result<Vec<Config>>>()?;
 
     let emitter: Box<dyn StatusEmitter + Send> = if env::var_os("CI").is_some() {
         Box::new((Text::verbose(), Gha::<true> { name: test_name.into() }))