search

la10736 / rstest

Fixture-based test framework for Rust
Apache License 2.0
1.21k stars 43 forks source link

PR #277 Follow-up #280

Closed hansl closed 1 month ago

hansl commented 1 month ago

@la10736

Just to keep a cleaner place to discuss the rerun-if-env-changed seen in https://github.com/la10736/rstest/pull/277

Here's what I did:

  1. I have a branch on the Boa project: https://github.com/boa-dev/boa/pull/4008. This uses ${WPT_ROOT} for the base dir of WPT tests.
  2. When I run this branch with the wpt-tests features (e.g. WPT_ROOT=$HOME/Sources/hansl/wpt cargo test -p boa_runtime --features "wpt-tests") it builds the tests and runs correctly.
  3. After that, I change the value of WPT_ROOT it will fail the tests if I put the wrong value (saying "file not found").
  4. If I remove the environment variable, it also fails (because the variable is needed when running the tests themselves, not just when building the test cases).
  5. If I change the variable to the correct value, the tests are passing again.

This proves as far as I know that the rerun-if-env-changed cargo instruction works at least in our case.

la10736 commented 1 month ago

Ok there's something wired... I guess. In branch https://github.com/la10736/rstest/tree/277_follow_up I add your code again and then changed playground crate to test it.

I also add a build.rs script gated by build-rs feature to add manually the env variable.

mdamico@miklap:~/devel/rstest/playground$ cargo test
   Compiling playground v0.1.0 (/home/mdamico/devel/rstest/playground)
cargo::rerun-if-env-changed=BASE_DIR
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.24s
     Running unittests src/main.rs (target/debug/deps/playground-da2bcf076a24a75e)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ BASE_DIR=pippo cargo test
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/main.rs (target/debug/deps/playground-da2bcf076a24a75e)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ cargo test --features build-rs
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/main.rs (target/debug/deps/playground-28552f639949a957)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ BASE_DIR=pippo cargo test --features build-rs
   Compiling playground v0.1.0 (/home/mdamico/devel/rstest/playground)
cargo::rerun-if-env-changed=BASE_DIR
error: Cannot canonicalize base dir due No such file or directory (os error 2)
 --> src/main.rs:8:5
  |
8 |     #[files("files/*.txt")]
  |     ^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::fs::File`
 --> src/main.rs:2:5
  |
2 | use std::fs::File;
  |     ^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `std::io::Read`
 --> src/main.rs:3:5
  |
3 | use std::io::Read;
  |     ^^^^^^^^^^^^^

warning: unused import: `std::path::PathBuf`
 --> src/main.rs:4:5
  |
4 | use std::path::PathBuf;
  |     ^^^^^^^^^^^^^^^^^^

warning: `playground` (bin "playground" test) generated 3 warnings
error: could not compile `playground` (bin "playground" test) due to 1 previous error; 3 warnings emitted
mdamico@miklap:~/devel/rstest/playground$ cargo test --features build-rs
   Compiling playground v0.1.0 (/home/mdamico/devel/rstest/playground)
cargo::rerun-if-env-changed=BASE_DIR
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.18s
     Running unittests src/main.rs (target/debug/deps/playground-28552f639949a957)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ cargo test
   Compiling playground v0.1.0 (/home/mdamico/devel/rstest/playground)
cargo::rerun-if-env-changed=BASE_DIR
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.24s
     Running unittests src/main.rs (target/debug/deps/playground-da2bcf076a24a75e)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ BASE_DIR=pippo cargo test
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/main.rs (target/debug/deps/playground-da2bcf076a24a75e)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ cargo test --features build-rs
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/main.rs (target/debug/deps/playground-28552f639949a957)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$ BASE_DIR=pippo cargo test --features build-rs
   Compiling playground v0.1.0 (/home/mdamico/devel/rstest/playground)
cargo::rerun-if-env-changed=BASE_DIR
error: Cannot canonicalize base dir due No such file or directory (os error 2)
 --> src/main.rs:8:5
  |
8 |     #[files("files/*.txt")]
  |     ^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::fs::File`
 --> src/main.rs:2:5
  |
2 | use std::fs::File;
  |     ^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `std::io::Read`
 --> src/main.rs:3:5
  |
3 | use std::io::Read;
  |     ^^^^^^^^^^^^^

warning: unused import: `std::path::PathBuf`
 --> src/main.rs:4:5
  |
4 | use std::path::PathBuf;
  |     ^^^^^^^^^^^^^^^^^^

warning: `playground` (bin "playground" test) generated 3 warnings
error: could not compile `playground` (bin "playground" test) due to 1 previous error; 3 warnings emitted
mdamico@miklap:~/devel/rstest/playground$ cargo test --features build-rs
   Compiling playground v0.1.0 (/home/mdamico/devel/rstest/playground)
cargo::rerun-if-env-changed=BASE_DIR
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.18s
     Running unittests src/main.rs (target/debug/deps/playground-28552f639949a957)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`

As you can see if I don't add build-rs feature also if define BASE_DIR=pippo tests are compiled correctly. Otherwise if I enable build-rs feature the compiler recompile the tests when the env is changed.

I didn't find any documentation about use rerun-if-*-changed syntax outside the build.rs script.

Now the question is.... Why is it working on your side?

la10736 commented 1 month ago

I also observed the same behavior if I move the tests outside (integration test).

Beside that we have also an annoying side effect on compile output:

   Compiling rstest_reuse v0.7.0 (/home/mdamico/devel/rstest/rstest_reuse)
   Compiling rstest v0.24.0-dev (/home/mdamico/devel/rstest/rstest)
cargo::rerun-if-env-changed=BASE_DIR
    Finished `test` profile [unoptimized + debuginfo] target(s) in 2.98s
     Running unittests src/main.rs (target/debug/deps/playground-da2bcf076a24a75e)

running 2 tests
test start_with_name::path_1_files_myname_txt ... ok
test start_with_name::path_2_files_should_fail_txt ... FAILED

failures:

---- start_with_name::path_2_files_should_fail_txt stdout ----
thread 'start_with_name::path_2_files_should_fail_txt' panicked at src/main.rs:17:5:
assertion failed: contents.starts_with(name.to_str().unwrap())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    start_with_name::path_2_files_should_fail_txt

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--bin playground`
mdamico@miklap:~/devel/rstest/playground$
la10736 commented 1 month ago

Ok, I understood why your case works: https://github.com/hansl/boa/blob/6c0abbc57e94b0a3a5b35517b68de85c27f2edf2/core/runtime/tests/wpt.rs#L216 also check the variable and it's executed for every test.

la10736 commented 1 month ago

Hi @hansl , can I close this ticket now?

hansl commented 1 month ago

I was able to verify the problem above and I agree with you. I'll come up with a solution, hopefully today. You can close this and I can open an issue for the bug itself.

la10736 commented 1 month ago

I don't think there's a solution that not involve the use of build script :cry: ... I've looked for something like this for a long time. Anyway I've already documented it.