fix(test2): Maintain fn path with #[test] macro#174
fix(test2): Maintain fn path with #[test] macro#174skogseth wants to merge 1 commit intoassert-rs:mainfrom
#[test] macro#174Conversation
Pull Request Test Coverage Report for Build 21962669234Details
💛 - Coveralls |
skogseth
force-pushed
the
dont-change-fn-path
branch
from
de4ad2a to
2a338ae
Compare
| (break: name=$name:ident body=[($($params:tt)*) $($item:tt)*] $(ignore=$ignore:tt)? $(should_panic=$should_panic:tt)?) => { | ||
| #[allow(non_camel_case_types)] | ||
| struct $name; | ||
| fn $name($($params)*) $($item)* |
There was a problem hiding this comment.
Is there a reason you also changed the structure?
There was a problem hiding this comment.
Yes. So since the name of the function is taken we need a unique path to whatever needs to implement the trait, so the only way I know to do that is to put it in a const block assigned to an anonymous constant. I'll write up a bit more about the design in the PR description
There was a problem hiding this comment.
Oh, I had misread what you meant with the title and thought this was #165 (forgot that was fixed).
There was a problem hiding this comment.
I do realize now that the Test type can conflict with a user defined type. Not sure what to do about that. I can sleep on it, maybe
There was a problem hiding this comment.
Nevermind, it's fine because the test function remains in the original scope, whilst the Test type is only accessible in the const-block scope.
skogseth
force-pushed
the
dont-change-fn-path
branch
from
2a338ae to
480f5b4
Compare
What end-user impact does this have though? btw I strongly prefer these kind of disucssions to happen in issues, not PRs. Maybe PRs come and go for a particular concern and that can split up design discussions. I'm also more likely to look into issues for these kinds of conversations. See also https://github.com/assert-rs/libtest2/blob/main/CONTRIBUTING.md#pull-requests btw this does move us closer to the shape of #148.
Oh nice, the MSRV problem was addressed. |
Relatively small I would assume? I have done this once ... I think.
...right. Yeah, sorry, I should have remembered that. I will try to remember in the future 👍
Yeah, seems like it |
3 participants
The standard test macro keeps the path for the test function the same, so you can also call it as a normal function. This behavior should probably be mimicked here.
Note: A nice little bonus is that the error messages for incorrect function argument types now correctly point at the function definition, and not the macro itself.
Design
We need a unique type to implement the
Casetrait on when adding a new test. Previously this used the unique identifier provided by the user in the function name, but this is now taken (by the function itself). We therefore make a new scope using aconst-block, which we just assign to an anonymous constant of type(). In this scope we can now define a new type, which we can implement the trait for.