Add framework to write detection tests

[luca3s]

Every file in tests/detection-tests/src/bin will be built and run with the sanitizer enabled when doing cargo test. On platforms without rtsan support these tests will be skipped and a warning will be printed. Currently the only thing that is being checked is, that the process exits unsuccesfully.

I had a patch that also allowed specifying a regex at the beginning of the file that would be checked against the process output, but to make this PR smaller i removed it again.

I have moved the detection tests to not be a part of the workspace anymore, because it felt weird to have a workspace member just for testing. This does make it a bit reliant on the precise paths of the folders and it leads to slower compilation as the target dir isn't shared anymore. If you would like it the other way better i can easily change it back.
Due to dependecy issues it is part of the workspace again.

I experimented a bit with Cargo script (which is currently nightly), but i couldn't find a way to make it work. I believe that in theory that could remove the need for a seperate crate.

I used libtest_mimic, it would maybe be possible to simplfy this by using datatest, which does the file detection automatically, but the code is pretty simple, so i thought this was better than an opaque macro, which datatest provides.

Would close #6

[steckes]

Very cool that works great!

1. I actually would have no problem with adding the test project to the workspace, example projects are also part of the workspace. I actually prefer having only one top level target folder, but I see that it still runs with cargo test from the root dir, so I don't have strong feelings about it.

2. Do you think we should run those tests only on supported architectures so cargo test succeeds on not-supported ones and at least tests that the macros can be inside of the code?

3. Should we name the tests more descriptive like manual_blocking/vector and maybe put them in a folder that is not named examples?

[luca3s]

I actually would have no problem with adding the test project to the workspace, example projects are also part of the workspace. I actually prefer having only one top level target folder, but I see that it still runs with cargo test from the root dir, so I don't have strong feelings about it.

Yeah my reasoning was that testing is internal and when we (at some future point) maybe test complicated things, a user shouldn't think that those really are examples on how the crate works.

Do you think we should run those tests only on supported architectures so cargo test succeeds on not-supported ones and at least tests that the macros can be inside of the code?

Sounds like a good idea! will look into this. Maybe i will print a warning or something like that.

Should we name the tests more descriptive like manual_blocking/vector and maybe put them in a folder that is not named examples?

I couldn't figure out how. I need some way to run the files while still providing them with the needed dependecies.

• I could create one binary crate per test, but that seems worse.
• I could do this with rustc directly (which is able to compile just one file), but then i need to do all the dependecy resolution and build script stuff myself, which sounds really complicated.
• I had hoped cargo script on nightly could do this, but the docs aren't that good yet and i couldn't figure out how. (also nightly maybe not so nice)

For renaming the tests themselves, sure i didn't put much thought into the names.

[luca3s]

Apparently the build script runs before my test runner runs, so i now check if rtsan is supported even if it's not enabled. Then i set rtsan_supported and check it later when running the tests. This has the big advantage that there is one source of truth and that is the build script.
I don't use println!("cargo::rustc-check-cfg=cfg as the cfg is for internal use only and using it should lead to warnings.

I am not able to test this in an unsupported environment (i don't want to setup a windows VM).

[luca3s]

Ohh yeah sorry about that. I added a dependency (libtest-mimic) but didn't update the Cargo.lock. Should be fixed now.

[steckes]

About the folder tests/detection/examples you can just rename the folder to bin and run the files with --bin instead of --example, that is a bit clearer

[luca3s]

About the folder tests/detection/examples you can just rename the folder to bin and run the files with --bin instead of --example, that is a bit clearer

That doesn't seem to compile. It tells me i need to add a [[bin]] section to the Cargo.toml for every binary target. This would kind of defeat the purpose of the automatic test detection and collection.

Ubunto fails because it doesn't have Rust 1.82, while i apparently upgraded to packages that require this. (That's what i gathered from the log) I will try to downgrade the cargo.lock a bit.

[steckes]

Ubunto fails because it doesn't have Rust 1.82, while i apparently upgraded to packages that require this. (That's what i gathered from the log) I will try to downgrade the cargo.lock a bit.

We can also update the MSRV to 1.82, I would switch to Rust 2024 edition anyways at one point which requires a newer version.

[steckes]

https://github.com/realtime-sanitizer/rtsan-standalone-rs/tree/stephan/detection-tests

Here I made it work without without specifying a [bin] section in the Cargo.toml, I guess because I put it in src/bin.

While playing a bit around with the code I think I prefer the following:

1. Renamed the files from detection to detection-tests as I prefer when the folder/files are called similar to the package/crate name.

2. Adding it to the workspace, as it is very clear here that this is only for tests.

[workspace]
members = ["crates/rtsan-standalone-sys", "tests/detection-tests"]

3. Just copied the examples manual_blocking and vector 1:1 including the file-names, as the integration example 1. doesn't exist anymore and has a lot of stuff in it that is not necessary for the test.

4. The command to run the tests then looks like this:

Command::new("cargo")
  .args(["run", "-p", "detection-tests", "--bin", &name])
  .env("RTSAN_ENABLE", "1")

For me this is a bit cleaner, how do you feel about those changes?

[luca3s]

We can also update the MSRV to 1.82, I would switch to Rust 2024 edition anyways at one point which requires a newer version.

If we switch to a workspace this isn't a problem anymore. I don't know why but apparently inside this inner crate Cargo chose a too new version of something and in the workspace it chose a correct version.

The changes you suggested sound good. I pulled from your branch to here. I also added a toolchain.toml to be sure that we really stay compatible with our MSRV and don't increase it on accident.

[luca3s]

One thing to be aware of: If any of the tests fail to compile they will be shown as a success. The way i noticed this usually was because the tests ran way too fast. The best way to solve this is probably checking the output of the test. In a earlier version of this i did this with regex and i will probably introduce it again with a second PR to not make this one bigger than it already is.

[luca3s]

Why are we using cargo update and then running cargo test --locked on the Ubuntu beta? Oh it is supposed to test the beta compiler ... then that kind of makes sense

info: note that the toolchain '1.81-x86_64-unknown-linux-gnu' is currently in use (overridden by '/home/runner/work/rtsan-standalone-rs/rtsan-standalone-rs/rust-toolchain.toml')

from the log output. So it uses 1.81 instead of beta. which is also why it fails. I will probably remove the toolchain file again.

[steckes]

Yeah we just have to change the github action to use the same MSRV that is contained in the main Cargo.toml

[luca3s]

Yeah we just have to change the github action to use the same MSRV that is contained in the main Cargo.toml

No i mean it makes sense to test with a new compiler. and when testing with a new compiler it also makes sense to update dependencies. That way you can be sure that it will continue to work when updating.
The issue was that i stopped it from using the new compiler by adding the rust-toolchain file, but it still did cargo update, so it had new dependecies with an old compiler.

[steckes]

Okay great. And the CI verifies now that the windows test is running and prints the warning.

[steckes]

Could you just remove the .gitignore entry that I forgot? Then I will merge it! :)