Tweaking test comments and usage of `.expect...`.
Based on earlier code review feedback, I am trying to ensure that all tests
(at least the ones touched during development of `cc_bindings_from_rs`)
consistently:
* Use proper doc comments (e.g. `/// ...` instead of `// ...`)
* Use `.unwrap()` and `.unwrap_err()` unless the message passed to
`.expect(...)` adds any signigicant information / value.
PiperOrigin-RevId: 495973452
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs
index 620c1d4..7526a4c 100644
--- a/cc_bindings_from_rs/bindings.rs
+++ b/cc_bindings_from_rs/bindings.rs
@@ -1125,11 +1125,14 @@
run_compiler(test_src, |tcx| find_def_id_by_name(tcx, "some_name"));
}
+ /// This test covers only a single example of a function that should get a
+ /// C++ binding. The test focuses on verification that the output from
+ /// `format_fn` gets propagated all the way to `GenerateBindings::new`.
+ /// Additional coverage of how functions are formatted is provided
+ /// by `test_format_def_..._fn_...` tests (which work at the `format_fn`
+ /// level).
#[test]
fn test_generated_bindings_fn_no_mangle_extern_c() {
- // This test covers only a single example of a function that should get a C++
- // binding. Additional coverage of how items are formatted is provided by
- // `test_format_def_..._fn_...` tests.
let test_src = r#"
#[no_mangle]
pub extern "C" fn public_function() {
@@ -1137,27 +1140,30 @@
}
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
assert_cc_matches!(
bindings.h_body,
quote! {
extern "C" void public_function();
}
);
- // TODO(b/254097223): Verify Rust thunks here (once they actually get generated).
+
+ // No Rust thunks should be generated in this test scenario.
+ assert_rs_not_matches!(bindings.rs_body, quote!{ public_function });
});
}
+ /// `test_generated_bindings_fn_export_name` covers a scenario where
+ /// `MixedSnippet::cc` is present but `MixedSnippet::rs` is empty
+ /// (because no Rust thunks are needed).
#[test]
fn test_generated_bindings_fn_export_name() {
- // Coverage of how `MixedSnippet::cc` are propagated when there is no
- // `MixedSnippet::rs` (e.g. no Rust thunks are needed).
let test_src = r#"
#[export_name = "export_name"]
pub extern "C" fn public_function(x: f64, y: f64) -> f64 { x + y }
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
assert_cc_matches!(
bindings.h_body,
quote! {
@@ -1174,20 +1180,20 @@
});
}
+ /// The `test_generated_bindings_struct` test covers only a single example
+ /// of an ADT (struct/enum/union) that should get a C++ binding.
+ /// Additional coverage of how items are formatted is provided by
+ /// `test_format_def_..._struct_...`, `test_format_def_..._enum_...`,
+ /// and `test_format_def_..._union_...` tests.
+ ///
+ /// We don't want to duplicate coverage already provided by
+ /// `test_format_def_struct_with_fields`, but we do want to verify that
+ /// * `format_crate` will actually find and process the struct
+ /// (`test_format_def_...` doesn't cover this aspect - it uses a test-only
+ /// `find_def_id_by_name` instead)
+ /// * The actual shape of the bindings still looks okay at this level.
#[test]
fn test_generated_bindings_struct() {
- // This test covers only a single example of an ADT (struct/enum/union) that
- // should get a C++ binding. Additional coverage of how items are
- // formatted is provided by `test_format_def_..._struct_...`,
- // `test_format_def_..._enum_...`, and `test_format_def_..._union_...`
- // tests.
- //
- // We don't want to duplicate coverage already provided by
- // `test_format_def_struct_with_fields`, but we do want to verify that
- // * `format_crate` will actually find and process the struct
- // (`test_format_def_...` doesn't cover this aspect - it uses a test-only
- // `find_def_id_by_name` instead)
- // * The overall shape of the bindings is present.
let test_src = r#"
pub struct Point {
pub x: i32,
@@ -1195,7 +1201,7 @@
}
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
assert_cc_matches!(
bindings.h_body,
quote! {
@@ -1231,7 +1237,7 @@
}
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
assert_cc_matches!(
bindings.h_body,
quote! {
@@ -1255,7 +1261,7 @@
pub struct S(bool);
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
assert_cc_matches!(
bindings.h_body,
quote! {
@@ -1299,7 +1305,7 @@
}
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
assert_cc_matches!(
bindings.h_body,
quote! {
@@ -1325,6 +1331,8 @@
});
}
+ /// `test_generated_bindings_non_pub_items` verifies that non-public items
+ /// are not present/propagated into the generated bindings.
#[test]
fn test_generated_bindings_non_pub_items() {
let test_src = r#"
@@ -1349,9 +1357,7 @@
}
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
-
- // Non-public items should not be present in the generated bindings.
+ let bindings = bindings.unwrap();
assert_cc_not_matches!(bindings.h_body, quote! { private_function });
assert_rs_not_matches!(bindings.rs_body, quote! { private_function });
assert_cc_not_matches!(bindings.h_body, quote! { PrivateStruct });
@@ -1371,7 +1377,7 @@
fn test_generated_bindings_top_level_items() {
let test_src = "pub fn public_function() {}";
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
let expected_comment_txt =
"Automatically @generated C++ bindings for the following Rust crate:\n\
rust_out";
@@ -1396,22 +1402,24 @@
})
}
+ /// The `test_generated_bindings_unsupported_item` test verifies how `Err`
+ /// from `format_def` is formatted as a C++ comment (in `format_crate`
+ /// and `format_unsupported_def`):
+ /// - This test covers only a single example of an unsupported item.
+ /// Additional coverage is provided by `test_format_def_unsupported_...`
+ /// tests.
+ /// - This test somewhat arbitrarily chooses an example of an unsupported
+ /// item, trying to pick one that 1) will never be supported (b/254104998
+ /// has some extra notes about APIs named after reserved C++ keywords) and
+ /// 2) tests that the full error chain is included in the message.
#[test]
fn test_generated_bindings_unsupported_item() {
- // This test verifies how `Err` from `format_def` is formatted as a C++ comment
- // (in `format_crate` and `format_unsupported_def`).
- // - This test covers only a single example of an unsupported item. Additional
- // coverage is provided by `test_format_def_unsupported_...` tests.
- // - This test somewhat arbitrarily chooses an example of an unsupported item,
- // trying to pick one that 1) will never be supported (b/254104998 has some extra
- // notes about APIs named after reserved C++ keywords) and 2) tests that the
- // full error chain is included in the message.
let test_src = r#"
#[no_mangle]
pub extern "C" fn reinterpret_cast() {}
"#;
test_generated_bindings(test_src, |bindings| {
- let bindings = bindings.expect("Test expects success");
+ let bindings = bindings.unwrap();
let expected_comment_txt = "Error generating bindings for `reinterpret_cast` \
defined at <crubit_unittests.rs>:3:17: 3:53: \
Error formatting function name: \
@@ -1445,14 +1453,17 @@
});
}
+ /// The `test_format_def_fn_explicit_unit_return_type` test below is very
+ /// similar to the
+ /// `test_format_def_fn_extern_c_no_mangle_no_params_no_return_type` above,
+ /// except that the return type is explicitly spelled out. There is no
+ /// difference in `ty::FnSig` so our code behaves exactly the same, but the
+ /// test has been planned based on earlier, hir-focused approach and having
+ /// this extra test coverage shouldn't hurt. (`hir::FnSig`
+ /// and `hir::FnRetTy` _would_ see a difference between the two tests, even
+ /// though there is no different in the current `bindings.rs` code).
#[test]
fn test_format_def_fn_explicit_unit_return_type() {
- // This test is very similar to the
- // `test_format_def_fn_extern_c_no_mangle_no_params_no_return_type` above, except that the
- // return type is explicitly spelled out. There is no difference in `ty::FnSig` so our
- // code behaves exactly the same, but the test has been planned based on earlier,
- // hir-focused approach and having this extra test coverage shouldn't hurt. (`hir::FnSig`
- // and `hir::FnRetTy` _do_ see a difference between the two tests).
let test_src = r#"
#[no_mangle]
pub extern "C" fn explicit_unit_return_type() -> () {}
@@ -1495,12 +1506,13 @@
})
}
+ /// `test_format_def_fn_mangling` checks that bindings can be generated for
+ /// `extern "C"` functions that do *not* have `#[no_mangle]` attribute. The
+ /// test elides away the mangled name in the `assert_cc_matches` checks
+ /// below, but end-to-end test coverage should eventually be provided by
+ /// `test/functions` (see b/262904507).
#[test]
fn test_format_def_fn_mangling() {
- // This test checks that bindings can be generated for `extern "C"` functions
- // that do *not* have `#[no_mangle]` attribute. The test elides away
- // the mangled name in the `assert_cc_matches` checks below, but
- // end-to-end test coverage is provided by `test/functions`.
let test_src = r#"
pub extern "C" fn public_function(x: f64, y: f64) -> f64 { x + y }
"#;
@@ -1548,13 +1560,12 @@
#[test]
fn test_format_def_unsupported_fn_unsafe() {
- // This tests how bindings for an `unsafe fn` are generated.
let test_src = r#"
#[no_mangle]
pub unsafe extern "C" fn foo() {}
"#;
test_format_def(test_src, "foo", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(
err,
"Bindings for `unsafe` functions \
@@ -1563,13 +1574,14 @@
});
}
+ /// `test_format_def_fn_const` tests how bindings for an `const fn` are
+ /// generated.
+ ///
+ /// Right now the `const` qualifier is ignored, but one can imagine that in the
+ /// (very) long-term future such functions (including their bodies) could
+ /// be translated into C++ `consteval` functions.
#[test]
fn test_format_def_fn_const() {
- // This tests how bindings for an `const fn` are generated.
- //
- // Right now the `const` qualifier is ignored, but one can imagine that in the
- // (very) long-term future such functions (including their bodies) could
- // be translated into C++ `consteval` functions.
let test_src = r#"
pub const fn foo(i: i32) -> i32 { i * 42 }
"#;
@@ -1768,7 +1780,7 @@
pub extern "C" fn reinterpret_cast() -> () {}
"#;
test_format_def(test_src, "reinterpret_cast", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(
err,
"Error formatting function name: \
@@ -1785,7 +1797,7 @@
pub extern "C" fn foo() -> *const i32 { std::ptr::null() }
"#;
test_format_def(test_src, "foo", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(
err,
"Error formatting function return type: \
@@ -1807,7 +1819,7 @@
// just call `no_bound_vars` on this `FnSig`'s `Binder`.
"#;
test_format_def(test_src, "foo", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Generics are not supported yet (b/259749023 and b/259749095)");
});
}
@@ -1822,7 +1834,7 @@
}
"#;
test_format_def(test_src, "generic_function", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Generics are not supported yet (b/259749023 and b/259749095)");
});
}
@@ -1836,7 +1848,7 @@
}
"#;
test_format_def(test_src, "Point", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Generics are not supported yet (b/259749023 and b/259749095)");
});
}
@@ -1850,7 +1862,7 @@
}
"#;
test_format_def(test_src, "Point", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Generics are not supported yet (b/259749023 and b/259749095)");
});
}
@@ -1864,7 +1876,7 @@
}
"#;
test_format_def(test_src, "SomeUnion", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Generics are not supported yet (b/259749023 and b/259749095)");
});
}
@@ -1875,7 +1887,7 @@
pub async fn async_function() {}
"#;
test_format_def(test_src, "async_function", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Error formatting function return type: \
The following Rust type is not supported yet: \
impl std::future::Future<Output = ()>");
@@ -1917,22 +1929,22 @@
});
}
+ /// `test_format_def_fn_rust_abi` tests a function call that is not a C-ABI, and
+ /// is not the default Rust ABI. It can't use `"stdcall"`, because it is
+ /// not supported on the targets where Crubit's tests run. So, it ended up
+ /// using `"vectorcall"`.
+ ///
+ /// This test almost entirely replicates `test_format_def_fn_rust_abi`, except
+ /// for the `extern "vectorcall"` part in the `test_src` test input.
+ ///
+ /// This test verifies the current behavior that gives reasonable and functional
+ /// FFI bindings. OTOH, in the future we may decide to avoid having the
+ /// extra thunk for cases where the given non-C-ABI function call
+ /// convention is supported by both C++ and Rust
+ /// (see also `format_cc_call_conv_as_clang_attribute` in
+ /// `rs_bindings_from_cc/src_code_gen.rs`)
#[test]
fn test_format_def_fn_vectorcall_abi() {
- // This tests a function call that is not a C-ABI, and is not the default Rust
- // ABI. It can't use "stdcall", because it is not supported on the
- // targets where Crubit's tests run. So, it ended up using
- // "vectorcall".
- //
- // This test almost entirely replicates `test_format_def_fn_rust_abi`, except
- // for the `extern "vectorcall"` part in the `test_src` test input.
- //
- // This test verifies the current behavior that gives reasonable and functional
- // FFI bindings. OTOH, in the future we may decide to avoid having the
- // extra thunk for cases where the given non-C-ABI function call
- // convention is supported by both C++ and Rust
- // (see also `format_cc_call_conv_as_clang_attribute` in
- // `rs_bindings_from_cc/src_code_gen.rs`)
let test_src = r#"
#![feature(abi_vectorcall)]
pub extern "vectorcall" fn add(x: f64, y: f64) -> f64 { x * y }
@@ -1974,7 +1986,7 @@
"#;
test_format_def(test_src, "variadic_function", |result| {
// TODO(b/254097223): Add support for variadic functions.
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "C variadic functions are not supported (b/254097223)");
});
}
@@ -2097,7 +2109,7 @@
pub extern "C" fn fn_with_params(_param: *const i32) {}
"#;
test_format_def(test_src, "fn_with_params", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Error formatting the type of parameter #0: \
The following Rust type is not supported yet: \
*const i32");
@@ -2111,7 +2123,7 @@
pub fn fn_with_params(_param: ()) {}
"#;
test_format_def(test_src, "fn_with_params", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Error formatting the type of parameter #0: \
`()` / `void` is only supported as a return type (b/254507801)");
});
@@ -2126,7 +2138,7 @@
pub extern "C" fn fn_with_params(_param: !) {}
"#;
test_format_def(test_src, "fn_with_params", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(
err,
"Error formatting the type of parameter #0: \
@@ -2246,7 +2258,7 @@
}
"#;
test_format_def(test_src, "reinterpret_cast", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(
err,
"Error formatting item name: \
@@ -2269,7 +2281,7 @@
}
"#;
test_format_def(test_src, "StructWithCustomDropImpl", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "`Drop` trait and \"drop glue\" are not supported yet (b/258251148)");
});
}
@@ -2293,14 +2305,14 @@
}
"#;
test_format_def(test_src, "StructRequiringCustomDropGlue", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "`Drop` trait and \"drop glue\" are not supported yet (b/258251148)");
});
}
- // This test covers how ZSTs (zero-sized-types) are handled.
- // https://doc.rust-lang.org/reference/items/structs.html refers to this kind of struct as a
- // "unit-like struct".
+ /// This test covers how ZSTs (zero-sized-types) are handled.
+ /// https://doc.rust-lang.org/reference/items/structs.html refers to this kind of struct as a
+ /// "unit-like struct".
#[test]
fn test_format_def_unsupported_struct_zero_sized_type() {
let test_src = r#"
@@ -2310,7 +2322,7 @@
"#;
for name in ["ZeroSizedType1", "ZeroSizedType2", "ZeroSizedType3"] {
test_format_def(test_src, name, |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Zero-sized types (ZSTs) are not supported (b/258259459)");
});
}
@@ -2430,7 +2442,7 @@
pub enum ZeroVariantEnum {}
"#;
test_format_def(test_src, "ZeroVariantEnum", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Zero-sized types (ZSTs) are not supported (b/258259459)");
});
}
@@ -2591,16 +2603,17 @@
pub static STATIC_VALUE: i32 = 42;
"#;
test_format_def(test_src, "STATIC_VALUE", |result| {
- let err = result.expect_err("Test expects an error here");
+ let err = result.unwrap_err();
assert_eq!(err, "Unsupported rustc_hir::hir::ItemKind: static item");
});
}
+ /// `test_format_ret_ty_for_cc_successes` provides test coverage for cases where
+ /// `format_ret_ty_for_cc` returns an `Ok(...)`. Additional testcases are
+ /// covered by `test_format_ty_for_cc_successes` (because
+ /// `format_ret_ty_for_cc` delegates most cases to `format_ty_for_cc`).
#[test]
fn test_format_ret_ty_for_cc_successes() {
- // Test coverage for cases where `format_ret_ty_for_cc` returns an `Ok(...)`.
- // Additional testcases are covered by `test_format_ty_for_cc_successes`
- // (because `format_ret_ty_for_cc` delegates most cases to `format_ty_for_cc`).
let testcases = [
// ( <Rust type>, <expected C++ type> )
("bool", "bool"), // TyKind::Bool
@@ -2620,15 +2633,18 @@
});
}
+ /// `test_format_ty_for_cc_successes` provides test coverage for cases where
+ /// `format_ty_for_cc` returns an `Ok(...)`.
+ ///
+ /// Note that using `std::int8_t` (instead of `::std::int8_t`) has been an
+ /// explicit decision. The "Google C++ Style Guide" suggests to "avoid
+ /// nested namespaces that match well-known top-level namespaces" and "in
+ /// particular, [...] not create any nested std namespaces.". It
+ /// seems desirable if the generated bindings conform to this aspect of the
+ /// style guide, because it makes things easier for *users* of these
+ /// bindings.
#[test]
fn test_format_ty_for_cc_successes() {
- // Test coverage for cases where `format_ty_for_cc` returns an `Ok(...)`.
- //
- // Using `std::int8_t` (instead of `::std::int8_t`) has been an explicit decision. The
- // "Google C++ Style Guide" suggests to "avoid nested namespaces that match well-known
- // top-level namespaces" and "in particular, [...] not create any nested std namespaces.".
- // It seems desirable if the generated bindings conform to this aspect of the style guide,
- // because it makes things easier for *users* of these bindings.
let testcases = [
// ( <Rust type>, (<expected C++ type>, <expected #include>, <expected prereq def>) )
("bool", ("bool", "", "")),
@@ -2700,28 +2716,28 @@
);
}
+ /// `test_format_ty_for_cc_failures` provides test coverage for cases where
+ /// `format_ty_for_cc` returns an `Err(...)`.
+ ///
+ /// It seems okay to have no test coverage for now for the following types
+ /// (which should never be encountered when generating bindings and where
+ /// `format_ty_for_cc` should panic):
+ /// - TyKind::Closure
+ /// - TyKind::Error
+ /// - TyKind::FnDef
+ /// - TyKind::Infer
+ ///
+ /// TODO(lukasza): Add test coverage (here and in the "for_rs" flavours) for:
+ /// - TyKind::Bound
+ /// - TyKind::Dynamic (`dyn Eq`)
+ /// - TyKind::Foreign (`extern type T`)
+ /// - https://doc.rust-lang.org/beta/unstable-book/language-features/generators.html:
+ /// TyKind::Generator, TyKind::GeneratorWitness
+ /// - TyKind::Param
+ /// - TyKind::Placeholder
+ /// - TyKind::Projection
#[test]
fn test_format_ty_for_cc_failures() {
- // This test provides coverage for cases where `format_ty_for_cc` returns an
- // `Err(...)`.
- //
- // TODO(lukasza): Add test coverage (here and in the "for_rs" flavours) for:
- // - TyKind::Bound
- // - TyKind::Dynamic (`dyn Eq`)
- // - TyKind::Foreign (`extern type T`)
- // - https://doc.rust-lang.org/beta/unstable-book/language-features/generators.html:
- // TyKind::Generator, TyKind::GeneratorWitness
- // - TyKind::Param
- // - TyKind::Placeholder
- // - TyKind::Projection
- //
- // It seems okay to have no test coverage for now for the following types (which
- // should never be encountered when generating bindings and where
- // `format_ty_for_cc` should panic):
- // - TyKind::Closure
- // - TyKind::Error
- // - TyKind::FnDef
- // - TyKind::Infer */
let testcases = [
// ( <Rust type>, <expected error message> )
(
diff --git a/cc_bindings_from_rs/cc_bindings_from_rs.rs b/cc_bindings_from_rs/cc_bindings_from_rs.rs
index e6b9764..2262730 100644
--- a/cc_bindings_from_rs/cc_bindings_from_rs.rs
+++ b/cc_bindings_from_rs/cc_bindings_from_rs.rs
@@ -429,10 +429,11 @@
Ok(())
}
+ /// `test_cmdline_error_propagation` tests that errors from `Cmdline::new` get
+ /// propagated. More detailed test coverage of various specific error types
+ /// can be found in tests in `cmdline.rs`.
#[test]
fn test_cmdline_error_propagation() -> anyhow::Result<()> {
- // Tests that errors from `Cmdline::new` get propagated. Broader coverage of
- // various error types can be found in tests in `cmdline.rs`.
let err = TestArgs::default_args()?
.with_extra_crubit_args(&["--unrecognized-crubit-flag"])
.run()
@@ -449,7 +450,6 @@
#[test]
fn test_rustc_error_propagation() -> anyhow::Result<()> {
- // Tests that `rustc` errors are propagated.
let err = TestArgs::default_args()?
.with_extra_rustc_args(&["--unrecognized-rustc-flag"])
.run()
@@ -460,11 +460,11 @@
Ok(())
}
+ /// `test_rustc_help` tests that we gracefully handle scenarios where `rustc`
+ /// doesn't compile anything (e.g. when there are no rustc cmdline
+ /// arguments, or when `--help` is present).
#[test]
fn test_rustc_help() -> anyhow::Result<()> {
- // Tests that we gracefully handle scenarios where `rustc` doesn't compile
- // anything (e.g. when there are no rustc cmdline arguments, or when
- // `--help` is present).
let err = TestArgs::default_args()?
.with_extra_rustc_args(&["--help"])
.run()
@@ -475,9 +475,10 @@
Ok(())
}
+ /// `test_rustc_unsupported_panic_mechanism` tests that `panic=unwind` results
+ /// in an error.
#[test]
fn test_rustc_unsupported_panic_mechanism() -> anyhow::Result<()> {
- // Tests that `panic=unwind` results in an error.
let err = TestArgs::default_args()?
.with_panic_mechanism("unwind")
.run()
@@ -488,10 +489,11 @@
Ok(())
}
+ /// `test_invalid_h_out_path` tests not only the specific problem of an invalid
+ /// `--h-out` argument, but also tests that errors from `run_with_tcx` are
+ /// propagated.
#[test]
fn test_invalid_h_out_path() -> anyhow::Result<()> {
- // Tests not only the specific problem of an invalid `--h-out` argument, but
- // also tests that errors from `run_with_tcx` are propagated.
let err = TestArgs::default_args()?
.with_h_path("../..")
.run()
@@ -502,9 +504,10 @@
Ok(())
}
+ /// `test_no_output_file` tests that we stop the compilation midway (i.e. that
+ /// we return `Stop` from `after_analysis`).
#[test]
fn test_no_output_file() -> anyhow::Result<()> {
- // Tests that we stop the compilation midway.
let tmpdir = tempdir()?;
let out_path = tmpdir.path().join("unexpected_output.o");
TestArgs::default_args()?
diff --git a/cc_bindings_from_rs/cmdline.rs b/cc_bindings_from_rs/cmdline.rs
index 6e961a5..bd0c56d 100644
--- a/cc_bindings_from_rs/cmdline.rs
+++ b/cc_bindings_from_rs/cmdline.rs
@@ -129,25 +129,28 @@
);
}
+ /// The `test_help` unit test below has multiple purposes:
+ /// - Direct/obvious purpose: testing that `--help` works
+ /// - Double-checking the overall shape of our cmdline "API" (i.e.
+ /// verification that the way we use `clap` attributes results in the
+ /// desired cmdline "API"). This is a good enough coverage to avoid having
+ /// flag-specifc tests (e.g. avoiding hypothetical
+ /// `test_h_out_missing_flag`, `test_h_out_missing_arg`,
+ /// `test_h_out_duplicated`).
+ /// - Exhaustively checking runtime asserts (assumming that tests run in a
+ /// debug build; other tests also trigger these asserts). See also:
+ /// - https://github.com/clap-rs/clap/issues/2740#issuecomment-907240414
+ /// - `clap::builder::App::debug_assert`
+ ///
+ /// To regenerate `expected_msg` do the following steps:
+ /// - Run `bazel run :cc_bindings_from_rs_legacy_toolchain_runner -- --help`
+ /// - Copy&paste the output of the command below
+ /// - Replace the 2nd `cc_bindings_from_rs` with
+ /// `cc_bindings_from_rs_unittest_executable`
#[test]
fn test_help() {
- // This test has multiple purposes:
- // - Direct/obvious purpose: testing that `--help` works
- // - Double-checking the overall shape of our cmdline "API" (i.e. verification that the way
- // we use `clap` attributes results in the desired cmdline "API"). This is a good enough
- // coverage to avoid having flag-specifc tests (e.g. avoiding hypothetical
- // `test_h_out_missing_flag`, `test_h_out_missing_arg`, `test_h_out_duplicated`).
- // - Exhaustively checking runtime asserts (assumming that tests run in a debug
- // build; other tests also trigger these asserts). See also:
- // - https://github.com/clap-rs/clap/issues/2740#issuecomment-907240414
- // - `clap::builder::App::debug_assert`
- //
- // To regenerate `expected_msg` do the following steps:
- // - Run `bazel run :cc_bindings_from_rs_legacy_toolchain_runner -- --help`
- // - Copy&paste the output of the command below
- // - Replace the 2nd `cc_bindings_from_rs` with `cc_bindings_from_rs_unittest_executable`
let anyhow_err = new_cmdline(["--help"]).expect_err("--help should trigger an error");
- let clap_err = anyhow_err.downcast::<clap::Error>().expect("Expecting `clap` error");
+ let clap_err = anyhow_err.downcast::<clap::Error>().unwrap();
let expected_msg = r#"cc_bindings_from_rs
Generates C++ bindings for a Rust crate
@@ -206,7 +209,7 @@
std::fs::write(&tmpfile, file_lines.as_slice().join("\n"))?;
let flag_file_arg = format!("@{}", tmpfile.display());
- let cmdline = new_cmdline([flag_file_arg.as_str()]).expect("No errors expected");
+ let cmdline = new_cmdline([flag_file_arg.as_str()]).unwrap();
assert_eq!(Path::new("foo.h"), cmdline.h_out);
assert_eq!(Path::new("foo_impl.rs"), cmdline.rs_out);
let rustc_args = &cmdline.rustc_args;
diff --git a/cc_bindings_from_rs/test/functions/functions_test.cc b/cc_bindings_from_rs/test/functions/functions_test.cc
index 9caae31..ac1b968 100644
--- a/cc_bindings_from_rs/test/functions/functions_test.cc
+++ b/cc_bindings_from_rs/test/functions/functions_test.cc
@@ -30,7 +30,7 @@
}
TEST(FunctionsTest, AddInt32ViaExternCWithMangling) {
- // TODO(b/258449205): Uncomment the test assertion below after ensuring that
+ // TODO(b/262904507): Uncomment the test assertion below after ensuring that
// the `genrule` in `test/functions/BUILD` invokes `cc_bindings_from_rs` with
// the same rustc cmdline flags as when `rustc` is used to build
// `functions.rs` for `rust_library`. Otherwise, the mangled name will be
diff --git a/common/code_gen_utils.rs b/common/code_gen_utils.rs
index 9f84dc3..aa8fdaf 100644
--- a/common/code_gen_utils.rs
+++ b/common/code_gen_utils.rs
@@ -279,7 +279,7 @@
#[test]
fn test_format_cc_ident_basic() {
assert_cc_matches!(
- format_cc_ident("foo").expect("No errors expected in this test"),
+ format_cc_ident("foo").unwrap(),
quote! { foo }
);
}
@@ -287,14 +287,14 @@
#[test]
fn test_format_cc_ident_reserved_rust_keyword() {
assert_cc_matches!(
- format_cc_ident("impl").expect("No errors expected in this test"),
+ format_cc_ident("impl").unwrap(),
quote! { impl }
);
}
#[test]
fn test_format_cc_ident_reserved_cc_keyword() {
- let err = format_cc_ident("reinterpret_cast").expect_err("This test expects an error");
+ let err = format_cc_ident("reinterpret_cast").unwrap_err();
let msg = err.to_string();
assert!(msg.contains("`reinterpret_cast`"));
assert!(msg.contains("C++ reserved keyword"));
@@ -303,7 +303,7 @@
#[test]
fn test_format_cc_ident_unfinished_group() {
let err = format_cc_ident("(foo") // No closing `)`.
- .expect_err("This test expects an error");
+ .unwrap_err();
let msg = err.to_string();
assert!(msg.contains("Can't format `(foo` as a C++ identifier"));
assert!(msg.contains("cannot parse"));
@@ -315,35 +315,35 @@
// These may appear in `IR::Func::name`.
assert_cc_matches!(
- format_cc_ident("operator==").expect("No errors expected in this test"),
+ format_cc_ident("operator==").unwrap(),
quote! { operator== }
);
assert_cc_matches!(
- format_cc_ident("operator new").expect("No errors expected in this test"),
+ format_cc_ident("operator new").unwrap(),
quote! { operator new }
);
// This may appear in `IR::Record::cc_name` (although in practice these will
// be namespace-qualified most of the time).
assert_cc_matches!(
- format_cc_ident("MyTemplate<int>").expect("No errors expected in this test"),
+ format_cc_ident("MyTemplate<int>").unwrap(),
quote! { MyTemplate<int> }
);
// These forms of unqualified identifiers are not used by Crubit in practice,
assert_cc_matches!(
- format_cc_ident("~MyClass").expect("No errors expected in this test"),
+ format_cc_ident("~MyClass").unwrap(),
quote! { ~MyClass }
);
assert_cc_matches!(
- format_cc_ident(r#" operator "" _km "#).expect("No errors expected in this test"),
+ format_cc_ident(r#" operator "" _km "#).unwrap(),
quote! { operator "" _km }
);
}
#[test]
fn test_format_cc_ident_empty() {
- let err = format_cc_ident("").expect_err("This test expects an error");
+ let err = format_cc_ident("").unwrap_err();
let msg = err.to_string();
assert_eq!(msg, "Empty string is not a valid C++ identifier");
}
@@ -354,7 +354,7 @@
// This may appear in `IR::Record::cc_name`.
assert_cc_matches!(
- format_cc_ident("std::vector<int>").expect("No errors expected in this test"),
+ format_cc_ident("std::vector<int>").unwrap(),
quote! { std::vector<int> }
);
}
@@ -487,7 +487,7 @@
let ns = create_namespace_qualifier_for_tests(&["foo", "reinterpret_cast", "bar"]);
let actual_rs = ns.format_for_rs();
assert_rs_matches!(actual_rs, quote! { foo :: reinterpret_cast :: bar :: });
- let cc_error = ns.format_for_cc().expect_err("This test expects an error");
+ let cc_error = ns.format_for_cc().unwrap_err();
let msg = cc_error.to_string();
assert!(msg.contains("`reinterpret_cast`"));
assert!(msg.contains("C++ reserved keyword"));
