fix: do not depend by default on async-std#311
fix: do not depend by default on async-std#311la10736 merged 13 commits intola10736:masterfrom coriolinus:303-no-async-std
async-std#311Conversation
This function emits the proper test attribute as required, or raises an error if the test is async and no explicit or implicit test attribute was found. This emits a compile error where the test is async and not explicit or implicit test attribute was found. However, this mode of error gives a relatively poor developer experience, so we'll want to add an appropriate error check for this case in the error-handling section of the macro as well.
…without test attr Note that this check only applies to tests, not to fixtures. A fixture shouldn't know or care whether it's being used in a sync or async context.
|
I need to add some tests but I think the core functionality here is in a relatively good state. Please let me know if you have any comments. |
Also fix docs and usage where necessary. Note that during the process of testing it was determined that we cannot support the trailing-exclamation format recommended by `macro_rules_attribute::apply`; that can't be parsed by `syn`, and it's not worth using a different parser for this case. That's ok, as `macro_rules_attribute::apply` appears to be willing to (begrudgingly) accept input lacking the trailing exclamation.
|
I now believe this to be ready for review. Looking forward to your comments @la10736 ! |
|
Hi @coriolinus , I'm little bit busy in these days. I hope to find some time next week. Anyway feel free to ping me if I'll forgot. |
…der`, set a test attr
The requirement that test attributes bind to the test and not the case was previously undocumented. This is much more relevant now that test attributes must be specified in the async case. However, this is _not_ a change in behavior; it always acted like this. It just wasn't written, previously.
|
@la10736 I've been working on this PR and at this point all test cases pass on my machine. Hopefully CI agrees! With regard to b5bf8e0, I don't love this limitation, but I don't see a way around it. I don't understand the attribute-parsing machinery well enough to improve on the situation. I do feel it's important to point out that this is not a new limitation introduced by this PR; it's just much more obvious now that every async case requires an explicit or implicit test attribute. |
First of all thanks very much to you great effort!!!! About this limitation, unfortunately there is no way to work around it in the general case without break a lot of tests around the world. Maybe change the rule of how to collect attribute for individual cases and all cases could be more intuitive in some cases like that but It also introduce a lot of wired cases. On the other side the attribute before and immediately after the But in this case we can decide to fetch and remove |
|
I'm looking at Are you willing to accept the PR in its current state, assuming that CI goes green? |
|
I took a rapid look to your code and the bigger issue is that you're not separating the parsing stage from the rendering one. You should try to do it like I did for the In your case you should check if there is one and only one Mainly I can merge accept this PR and fix it later, but I prefer to wait when I can find some time to do it. |
|
In my last comment I forgot to mention that |
|
Ok, I merge it. Now I'm working on it to make it little bit more consistent with the rest of the code. Really thanks again!! |
|
Thank you! |
Closes #303.