ctest: Add macro expansion and ctest entrypoint. #4484
Conversation
tgross35
left a comment
tgross35
left a comment
There was a problem hiding this comment.
Looks pretty good, some small requests. Please also add a test file next to simple.rs that puts some code in a macro.
|
|
||
| use crate::expand; | ||
|
|
||
| pub struct TestGenerator; |
There was a problem hiding this comment.
Make this #[non_exhaustive] struct TestGenerator {} so building this has to go through the constructor
ctest-next/src/generation/mod.rs
Outdated
There was a problem hiding this comment.
Can you remove the module nesting and just move src/generation/generator.rs to src/generator.rs? I don't think we're going to have more than one file here.
There was a problem hiding this comment.
Enforce some rules from the get go
#![warn(missing_docs)]
#![warn(unreachable_pub)]
ctest-next/tests/basic.rs
Outdated
| generator.generate("./tests/sources/hierarchy/lib.rs", "hierarchy_out.rs")?; | ||
| generator.generate("./tests/sources/simple.rs", "simple_out.rs")?; |
There was a problem hiding this comment.
? -> .unwrap() so we get the information about which line failed. The test can drop the -> Result<...> return type.
Could you also split these test sources into two different #[test] functions?
There was a problem hiding this comment.
//! Ensure that our crate is able to handle definitions spread across many files
| &self, | ||
| crate_path: P, | ||
| _output_file_path: P, | ||
| ) -> Result<(), Box<dyn Error>> { |
There was a problem hiding this comment.
Could you add
type Error = Box<dyn std::error::Error>;
type Result<T, E = Error> = std::result::Result<T, E>;And then import those? Rather than retyping the definitions everywhere.
tgross35
left a comment
tgross35
left a comment
There was a problem hiding this comment.
A few remaining nits then please squash and this should be good to merge
ctest-next/src/error.rs
Outdated
There was a problem hiding this comment.
Anything tiny like this can exist in the parent module, no need for a separate file. It can move if we wind up with more error types but we might not.
ctest-next/tests/sources/macro.rs
Outdated
| macro_rules! t { | ||
| ($e:expr) => { | ||
| match $e { | ||
| Ok(e) => e, | ||
| Err(e) => panic!("{} failed with {}", stringify!($e), e), | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| pub fn write_buffer() { | ||
| let buf = String::new(); | ||
| t!(writeln!(buf, "#define A 1")); | ||
| } |
There was a problem hiding this comment.
Could the macro generate structs or functions that will be visible in the C interface? Since that is what we'll want to test.
| @@ -0,0 +1,11 @@ | |||
| pub type Byte = u8; | |||
|
|
|||
| pub struct Person { | |||
There was a problem hiding this comment.
#[repr(C)] since we'll want to have it exported
ctest-next/tests/sources/macro.rs
Outdated
There was a problem hiding this comment.
Thinking about it some, could you rename the sources directory to input? Probably makes things a bit more obvious.
|
CI also needs to be fixed, probably an update here Lines 97 to 99 in 2d96246 |
|
The |
Could you add a test with invalid syntax in Please squash the commits once this is all set, everything else lgtm |
mbyx
force-pushed
the
ctest_macro_expansion
branch
from
d029284 to
7b1b6c3
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
tgross35
changed the title
tgross35
added
the
stable-unneeded
3 participants
Description
This PR adds a function to expand a crate (consisting of either a single file or a folder) using rustc, a stub entrypoint for test generation, and a simple test that checks whether it can expand both types of crates.
Sources
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI