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

PR #277 Follow-up #280

Closed
hansl opened this issue Sep 30, 2024 · 6 comments
Closed

PR #277 Follow-up #280

hansl opened this issue Sep 30, 2024 · 6 comments

Comments

@hansl
Copy link
Contributor

hansl commented Sep 30, 2024

@la10736

Just to keep a cleaner place to discuss the rerun-if-env-changed seen in #277

Here's what I did:

  1. I have a branch on the Boa project: Add WPT as optional tests for boa_runtime boa-dev/boa#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
Copy link
Owner

la10736 commented Sep 30, 2024

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
Copy link
Owner

la10736 commented Sep 30, 2024

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
Copy link
Owner

la10736 commented Sep 30, 2024

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
Copy link
Owner

la10736 commented Oct 1, 2024

Hi @hansl ,
can I close this ticket now?

@hansl
Copy link
Contributor Author

hansl commented Oct 1, 2024

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
Copy link
Owner

la10736 commented Oct 1, 2024

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

@la10736 la10736 closed this as completed Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants