dtolnay / request-for-implementation

Crates that don't exist, but should
610 stars 6 forks source link

Testing tool for comparing output of macro against expected source code #28

Closed dtolnay closed 4 years ago

dtolnay commented 5 years ago

Similar to what https://github.com/dtolnay/trybuild does for testing compilation failures.

See here and here for a basic test setup leveraging trybuild.

I would expect a very similar setup for testing macro expansion -- with a suite of test cases as *.rs files in some directory, adjacent to expected expanded output in *.expanded.rs files with corresponding names. For each test case the library would set up dependencies and stuff correctly then invoke cargo expand to expand macros and format the expanded code with rustfmt. Then the test case would fail if the expanded and formatted code differs from the expected output in the corresponding *.expanded.rs file.

dtolnay commented 5 years ago

This is better than invoking proc macros directly as fn(TokenStream) -> TokenStream from tests for a few reasons:

eupn commented 4 years ago

@dtolnay I want to give it a try. It's ok?

dtolnay commented 4 years ago

Sounds good! Thanks for taking a look.

eupn commented 4 years ago

@dtolnay macrotest repository is here: https://github.com/eupn/macrotest. I have quick-and-dirty version done and simple macro_rules! macro expansion test project here: https://github.com/eupn/macrotest/tree/master/test-project/tests, check it out.

eupn commented 4 years ago

Oh, I see that you've updated the issue's description. I will update the macrotest's API to resemble the trybuild closely.

eupn commented 4 years ago

Updated the API to be similar to trybuild. https://github.com/eupn/macrotest/blob/master/test-project/tests/tests.rs

@dtolnay what should be considered as "expected failure" case of cargo expand invocation? Any non-success return code? Or should we add a comparison with .stderr file?

dtolnay commented 4 years ago

There are no expected failures. If any cargo expand exits unsuccessful then it should fail the test.

eupn commented 4 years ago

Published on crates.io: macrotest

eupn commented 4 years ago

Added procedural macro example. Even though it has some issues (#6, #7), it works.

dtolnay commented 4 years ago

Nice, looks like a great start. I filed some issues based on trying out the crate. I think we can close out this issue once those are fixed. Thanks!

eupn commented 4 years ago

Thanks for the thorough review! Will fix as many as possible.

eupn commented 4 years ago

@dtolnay issues are fixed

dtolnay commented 4 years ago

Awesome!

eupn commented 4 years ago

@dtolnay btw, which of your crates with macros/proc-macros (or crates that you know) would make use of this tool? I'd be happy to file a PR to some of those crates to employ macrotest in them.

dtolnay commented 4 years ago

I would like to try it in Serde because the logic inside Deserialize derives can be pretty complicated to reason about. https://github.com/serde-rs/serde/issues/1117