Support replacing the RustTest class and factory for Rust browser tests [chromium/src : main]
danakj (Gerrit)
Attention is currently required from: Łukasz Anforowicz.
danakj uploaded patch set #12 to this change.
Support replacing the RustTest class and factory for Rust browser tests
Browser tests need two things from their C++ test suite class:
1) Subclass BrowserTestBase instead of testing::Test. This is supported
already by the #[extern_test_suite] macro which is used to implement
the rust_gtest_interop::TestSuite trait for a Rust wrapper type of a
C++ subclass of testing::Test.
2) Run the Rust test function from an override of
BrowserTestBase::RunTestOnMainThread(). Usually gtests run the Rust
test function in the TestBody() override, and this is what is done by
the rust_gtest_interop::RustTest C++ class. But browser tests don't use
this method. So the RustTest class, and its factory, must be replaced
for browser tests.
This CL provides a way to replace the RustTest class, its factory, and
the RUST_GTEST_TEST_SUITE_FACTORY() macro with your own. The //content
browser test support libraries will do this to provide a framework for
writing Rust browser tests.
It is done by extending the #[extern_test_suite] macro to allow over-
riding the text string embedded in the RUST_GTEST_TEST_SUITE_FACTORY()
macro, which points to the factory function that constructs the
RustTest class.
Bug: 1305396
Change-Id: Ia87ab5856260f9edc9c211804a5af221e63058f7
Cq-Include-Trybots: luci.chromium.try:linux-rust-x64-rel,linux-rust-x64-dbg,android-rust-arm-rel,android-rust-arm-dbg
---
A content/browser/smoke_rust_browsertest.rs
A content/public/test/content_browser_test.rs
A content/test/browser_test_macro.rs
A content/test/content_public_browsertest_support.rs
A content/test/content_public_test_support.rs
M testing/rust_gtest_interop/gtest_attribute.rs
M testing/rust_gtest_interop/rust_gtest_interop.h
M testing/rust_gtest_interop/rust_gtest_interop_unittest.rs
M testing/rust_gtest_interop/rust_gtest_interop_unittest_main.cc
M testing/rust_gtest_interop/test/test_subclass.cc
M testing/rust_gtest_interop/test/test_subclass.h
M testing/rust_gtest_interop/test/test_subclass.rs
12 files changed, 269 insertions(+), 33 deletions(-)
To view, visit change 3644893. To unsubscribe, or for help writing mail filters, visit settings.
danakj (Gerrit)
Attention is currently required from: Łukasz Anforowicz.
2 comments:
Patchset:
Unresolving
Done
Patchset:
PTAL :)
To view, visit change 3644893. To unsubscribe, or for help writing mail filters, visit settings.
danakj (Gerrit)
Attention is currently required from: Łukasz Anforowicz.
1 comment:
Patchset:
Android build is unhappy with using autocxx due to some sysroot problem that ade is investigating. If we can't resolve that I will revert to using cxx there for now.
To view, visit change 3644893. To unsubscribe, or for help writing mail filters, visit settings.
danakj (Gerrit)
Patchset:
Back to using cxx instead of autocxx, since it's not happy on android.
To view, visit change 3644893. To unsubscribe, or for help writing mail filters, visit settings.
Łukasz Anforowicz (Gerrit)
Attention is currently required from: danakj.
Patch set 16:Code-Review +1
3 comments:
Patchset:
Thanks! LGTM. FWIW, the internals seem quite tricky to get right, but I don't see any better alternatives + in practice there will be a limited number of test suite base classes in use (and once the scaffolding is in place, then actually using a test suite isn't that bad I think...).
File testing/rust_gtest_interop/gtest_attribute.rs:
Ok(x) => Err(x.span()),
Err(x) => Err(x.span()),nit: Not sure if merging these 2 lines into 1 is more readable, but maybe this is something to consider:
Ok(x) | Err(x) => Err(x.span()),
Patch Set #16, Line 395:
or_elsenit: `Result::or_else` works, but it feels that `Result::map_err` is closer to what this code accomplishes.
To view, visit change 3644893. To unsubscribe, or for help writing mail filters, visit settings.
danakj (Gerrit)
2 comments:
File testing/rust_gtest_interop/gtest_attribute.rs:
Ok(x) => Err(x.span()),
Err(x) => Err(x.span()),
nit: Not sure if merging these 2 lines into 1 is more readable, but maybe this is something to consi […]
Can't because the `x` are different types 😊
Patch Set #16, Line 395:
or_else
nit: `Result::or_else` works, but it feels that `Result::map_err` is closer to what this code accomp […]
Yeah! Thanks
To view, visit change 3644893. To unsubscribe, or for help writing mail filters, visit settings.
danakj (Gerrit)
Patch set 17:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
16 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: testing/rust_gtest_interop/gtest_attribute.rs
Insertions: 3, Deletions: 3.
@@ -392,12 +392,12 @@
Ok(x) => Err(x.span()),
Err(x) => Err(x.span()),
};
- parsed.or_else(|span| {
- Err(quote_spanned! { span =>
+ parsed.map_err(|span| {
+ quote_spanned! { span =>
compile_error!(
"invalid syntax for extern_test_suite macro, \
expected `#[cpp_prefix("PREFIX_STRING_")]`");
- })
+ }
})
}
```
Support replacing the RustTest class and factory for Rust browser tests
Browser tests need two things from their C++ test suite class:
1) Subclass BrowserTestBase instead of testing::Test. This is supported
already by the #[extern_test_suite] macro which is used to implement
the rust_gtest_interop::TestSuite trait for a Rust wrapper type of a
C++ subclass of testing::Test.
2) Run the Rust test function from an override of
BrowserTestBase::RunTestOnMainThread(). Usually gtests run the Rust
test function in the TestBody() override, and this is what is done by
the rust_gtest_interop::RustTest C++ class. But browser tests don't use
this method. So the RustTest class, and its factory, must be replaced
for browser tests.
This CL provides a way to replace the RustTest class, its factory, and
the RUST_GTEST_TEST_SUITE_FACTORY() macro with your own. The //content
browser test support libraries will do this to provide a framework for
writing Rust browser tests.
It is done by extending the #[extern_test_suite] macro to allow over-
riding the text string embedded in the RUST_GTEST_TEST_SUITE_FACTORY()
macro, which points to the factory function that constructs the
RustTest class.
Bug: 1305396
Change-Id: Ia87ab5856260f9edc9c211804a5af221e63058f7
Cq-Include-Trybots: luci.chromium.try:linux-rust-x64-rel,linux-rust-x64-dbg,android-rust-arm-rel,android-rust-arm-dbg
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3644893
Reviewed-by: Łukasz Anforowicz <luk...@chromium.org>
Commit-Queue: danakj <dan...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1003441}
---
M testing/rust_gtest_interop/gtest_attribute.rs
M testing/rust_gtest_interop/rust_gtest_interop.h
M testing/rust_gtest_interop/rust_gtest_interop_unittest.rs
M testing/rust_gtest_interop/rust_gtest_interop_unittest_main.cc
M testing/rust_gtest_interop/test/test_subclass.cc
M testing/rust_gtest_interop/test/test_subclass.h
M testing/rust_gtest_interop/test/test_subclass.rs
7 files changed, 197 insertions(+), 26 deletions(-)