Skip to content

Commit

Permalink
Disallow !Sync types in #[once] fixtures (#237)
Browse files Browse the repository at this point in the history 
* disallow `!Sync` types in `#[once]` fixtures

* Dont use `once_cell` use std::sync::OnceLock instead

* add changelog entry for #237

---------

Co-authored-by: la10736 <michele.damico@gmail.com>
  • Loading branch information
@narpfel @la10736
narpfel and la10736 committed Apr 3, 2024
1 parent b32e182 commit 3f91774
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 128 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -8,6 +8,11 @@

### Fixed

- `#[once]` fixtures now require the returned type to be
[`Sync`](https://doc.rust-lang.org/std/marker/trait.Sync.html) to prevent UB
when tests are executed in parallel. (see [#235](https://github.com/la10736/rstest/issues/235)
for more details)

## [0.18.2] 2023/8/13

### Changed
Expand Down
285 changes: 178 additions & 107 deletions rstest/tests/fixture/mod.rs
View file
Expand Up @@ -221,131 +221,202 @@ mod should {
assert_eq!(1, occurences);
}

#[test]
fn show_correct_errors() {
let prj = prj("errors.rs");
let output = prj.run_tests().unwrap();
let name = prj.get_name();
mod show_correct_errors {
use super::*;
use std::process::Output;

assert_in!(output.stderr.str(), "error[E0433]: ");
assert_in!(
output.stderr.str(),
format!(
r#"
--> {}/src/lib.rs:12:33
|
12 | fn error_cannot_resolve_fixture(no_fixture: u32) {{"#,
name
)
.unindent()
);
use rstest::{fixture, rstest};

assert_in!(
output.stderr.str(),
format!(
r#"
error[E0308]: mismatched types
--> {}/src/lib.rs:8:18
|
8 | let a: u32 = "";
"#,
name
)
.unindent()
);
#[fixture]
#[once]
fn errors_rs() -> (Output, String) {
run_test("errors.rs")
}

assert_in!(
output.stderr.str(),
format!(
"
error[E0308]: mismatched types
--> {}/src/lib.rs:16:29
",
name
)
.unindent()
);
assert_in!(
output.stderr.str(),
"
16 | fn error_fixture_wrong_type(fixture: String) {
| ^^^^^^"
#[rstest]
fn when_cannot_resolve_fixture(errors_rs: &(Output, String)) {
let (output, name) = errors_rs.clone();

assert_in!(output.stderr.str(), "error[E0433]: ");
assert_in!(
output.stderr.str(),
format!(
r#"
--> {}/src/lib.rs:12:33
|
12 | fn error_cannot_resolve_fixture(no_fixture: u32) {{"#,
name
)
.unindent()
);
);
}

assert_in!(
output.stderr.str(),
format!(
#[rstest]
fn on_mismatched_types_inner(errors_rs: &(Output, String)) {
let (output, name) = errors_rs.clone();

assert_in!(
output.stderr.str(),
format!(
r#"
error[E0308]: mismatched types
--> {}/src/lib.rs:8:18
|
8 | let a: u32 = "";
"#,
name
)
.unindent()
);
}

#[rstest]
fn on_mismatched_types_argument(errors_rs: &(Output, String)) {
let (output, name) = errors_rs.clone();

assert_in!(
output.stderr.str(),
format!(
"
error[E0308]: mismatched types
--> {}/src/lib.rs:16:29
",
name
)
.unindent()
);

assert_in!(
output.stderr.str(),
"
error: Missed argument: 'not_a_fixture' should be a test function argument.
--> {}/src/lib.rs:19:11
|
19 | #[fixture(not_a_fixture(24))]
| ^^^^^^^^^^^^^
",
name
)
.unindent()
);
16 | fn error_fixture_wrong_type(fixture: String) {
| ^^^^^^"
.unindent()
);
}

assert_in!(
output.stderr.str(),
format!(
r#"
error: Duplicate argument: 'f' is already defined.
--> {}/src/lib.rs:33:23
|
33 | #[fixture(f("first"), f("second"))]
| ^
"#,
name
)
.unindent()
);
#[rstest]
fn on_invalid_fixture(errors_rs: &(Output, String)) {
let (output, name) = errors_rs.clone();

assert_in!(
output.stderr.str(),
format!(
"
error: Missed argument: 'not_a_fixture' should be a test function argument.
--> {}/src/lib.rs:19:11
|
19 | #[fixture(not_a_fixture(24))]
| ^^^^^^^^^^^^^
",
name
)
.unindent()
);
}

assert_in!(
output.stderr.str(),
format!(
r#"
error: Cannot apply #[once] to async fixture.
--> {}/src/lib.rs:38:3
|
38 | #[once]
| ^^^^
"#,
name
)
.unindent()
);
#[rstest]
fn on_duplicate_fixture_argument(errors_rs: &(Output, String)) {
let (output, name) = errors_rs.clone();

assert_in!(
output.stderr.str(),
format!(
r#"
error: Duplicate argument: 'f' is already defined.
--> {}/src/lib.rs:33:23
|
33 | #[fixture(f("first"), f("second"))]
| ^
"#,
name
)
.unindent()
);
}

assert_in!(
output.stderr.str(),
format!(
r#"
error: Cannot apply #[once] on generic fixture.
--> {}/src/lib.rs:43:3
|
43 | #[once]
| ^^^^
"#,
name
)
.unindent()
);

assert_in!(
#[fixture]
#[once]
fn errors_once_rs() -> (Output, String) {
run_test("errors_once.rs")
}

#[rstest]
fn once_async(errors_once_rs: &(Output, String)) {
let (output, name) = errors_once_rs.clone();
assert_in!(
output.stderr.str(),
format!(
r#"
error: Cannot apply #[once] to async fixture.
--> {}/src/lib.rs:4:3
|
4 | #[once]
| ^^^^
"#,
name
)
.unindent()
);
}

#[rstest]
fn once_generic_type(errors_once_rs: &(Output, String)) {
let (output, name) = errors_once_rs.clone();
assert_in!(
output.stderr.str(),
format!(
r#"
error: Cannot apply #[once] on generic fixture.
--> {}/src/lib.rs:9:3
|
9 | #[once]
| ^^^^
"#,
name
)
.unindent()
);
}

#[rstest]
fn once_generic_impl(errors_once_rs: &(Output, String)) {
let (output, name) = errors_once_rs.clone();
assert_in!(
output.stderr.str(),
format!(
r#"
error: Cannot apply #[once] on generic fixture.
--> {}/src/lib.rs:49:3
--> {}/src/lib.rs:15:3
|
49 | #[once]
15 | #[once]
| ^^^^
"#,
name
)
.unindent()
);
}
);

}

#[rstest]
fn once_on_not_sync_type(errors_once_rs: &(Output, String)) {
let (output, name) = errors_once_rs.clone();
assert_in!(
output.stderr.str(),
format!(
r#"
error[E0277]: `Cell<u32>` cannot be shared between threads safely
--> {}/src/lib.rs:20:1
|
20 | #[fixture]
| ^^^^^^^^^^ `Cell<u32>` cannot be shared between threads safely
"#,
name,
)
.unindent(),
);
}
}
}
17 changes: 0 additions & 17 deletions rstest/tests/resources/fixture/errors.rs
View file
Expand Up @@ -33,20 +33,3 @@ fn f(name: &str) -> String {
#[fixture(f("first"), f("second"))]
fn error_inject_a_fixture_more_than_once(f: String) {
}

#[fixture]
#[once]
async fn error_async_once_fixture() {
}

#[fixture]
#[once]
fn error_generics_once_fixture<T: std::fmt::Debug>() -> T {
42
}

#[fixture]
#[once]
fn error_generics_once_fixture() -> impl Iterator<Item: u32> {
std::iter::once(42)
}
24 changes: 24 additions & 0 deletions rstest/tests/resources/fixture/errors_once.rs
View file
@@ -0,0 +1,24 @@
use rstest::*;

#[fixture]
#[once]
async fn error_async_once_fixture() {
}

#[fixture]
#[once]
fn error_generics_once_fixture<T: std::fmt::Debug>() -> T {
42
}

#[fixture]
#[once]
fn error_generics_once_fixture() -> impl Iterator<Item = u32> {
std::iter::once(42)
}

#[fixture]
#[once]
fn error_once_fixture_not_sync() -> std::cell::Cell<u32> {
std::cell::Cell::new(42)
}
7 changes: 3 additions & 4 deletions rstest_macros/src/render/fixture.rs
View file
Expand Up @@ -21,10 +21,9 @@ fn wrap_return_type_as_static_ref(rt: ReturnType) -> ReturnType {
fn wrap_call_impl_with_call_once_impl(call_impl: TokenStream, rt: &ReturnType) -> TokenStream {
match rt {
syn::ReturnType::Type(_, t) => parse_quote! {
static mut S: Option<#t> = None;
static CELL: std::sync::Once = std::sync::Once::new();
CELL.call_once(|| unsafe { S = Some(#call_impl) });
unsafe { S.as_ref().unwrap() }
static CELL: std::sync::OnceLock<#t> =
std::sync::OnceLock::new();
CELL.get_or_init(|| #call_impl )
},
_ => parse_quote! {
static CELL: std::sync::Once = std::sync::Once::new();
Expand Down

0 comments on commit 3f91774

Please sign in to comment.