Proc Macro Attributes

[awestlake87]

I mentioned in the guide PR that we can't use the convenient proc macros for rust runtimes like #[async_std::main] or #[tokio::test] in pyo3-asyncio, however that doesn't mean we can't make these proc macros for pyo3-asyncio.

I think once this crate gets published this feature will probably be requested, so I figured I'd just open it up so we can start having a discussion on it.

Proc Macro Attribute for the Main function

This one is fairly straightforward. We basically just need to expand it to the initialization example in the README. The only problem is that the initialization depends heavily on the Rust runtime. We could allow the runtime to be specified via #[pyo3_asyncio::main(rt = tokio)], but each runtime might have their own configuration options (multi-threaded, thread-count, etc) so my personal preference would be to provide these attributes per runtime submodule like this: #[pyo3_asyncio::tokio::main] and handle the configuration separately.

Proc Macro Attribute for Tests

I believe this one is way trickier. Unfortunately, I also believe it's gonna be the most requested. What we have set up for the PyO3 asyncio test harness is functional, but it's also pretty tedious. Being able to specify tests with an attribute, and even better, being able to have unit tests inside the library code is really nice. I would like to be able to set something up like the #[wasm_bindgen_test] attribute, but I currently don't know how to do that.

I might investigate this a bit further soon, but in the meantime, if anyone has any experience with something like this, feel free to comment on this thread or point me to some links. Otherwise, I'll poke around in the wasm-bindgen code a bit.

Like the main function, this will be specific to the runtime they've selected, so these attributes will need to go in the corresponding submodules. This could pose some issues for people who might want to test for multiple runtimes as I'm not sure if we could do multiple passes over the lib tests to run the set of async-std tests, then the tokio tests, etc. Could we run all of the rust runtimes concurrently in the test harness?

Update: At a glance, it seems like one potential avenue is to create an extern "C" symbol for each test function prefixed by something like pyt_*. If we could somehow iterate over the symbols in the test binary via dlsym or something similar, we could filter out the ones prefixed with pyt_* and add them to the list we pass into the test harness.

[awestlake87]

Good news on this front! I found the inventory crate, which allows us to generate a list of structures at compile time using macros. This way, we can create a list of tests using proc macros and iterate over them in the main function. I've just pushed a set of changes that add support for #[pyo3_asyncio::async_std::test] and pyo3_asyncio::async_std::test_main!("suite name").

This allows us to generate the list of tests using proc macros, but still requires us to override the default test harness, provide our own main function, and it can only be used for integration tests. I think these are pretty hardline limitations, though. I don't know if there is a way to get around them.

Additionally, users must specify the inventory crate as a dev dependency. However, I personally prefer this solution over the one we had before. If anyone has any feedback I'd be happy to hear it though!

[awestlake87]

I think the initial design is complete. All existing tests that use the test harness have been converted over to the proc macros and the examples in the docs are converted too.

One problem that still stands is that the doctests don't think that the test_main! macro is providing the main fn (even though it is), so it wraps the test in a main function and screws up the resolution of the Test struct in the macro expansion. The Test struct has to be a member of the crate where the test functions are defined (a restriction of the inventory crate), so I can't move it into pyo3-asyncio unfortunately. Idk if there's a way to disable that main wrapping in doctests or not aside from defining main inside the test.

This could also be problematic if someone calls test_main! outside of the crate root, but I've never personally seen anyone do that with the main fn.

[awestlake87]

In addition to integration tests, it seems that doctests will also run if you use the #[main] proc macros. It seems obvious now that I think about it since they have control over fn main(), but I just hadn't considered it. I went ahead and removed the no_run flags on all of the doctests I could and added some sections to the testing module mentioning doctests and how they should be set up / the caveats involved.

Also, the issue I mentioned in my previous comment has a workaround that I documented and applied in the testing module. Seems like it might be a bug with Rust's doctest implementation, but since it has a pretty straightforward workaround, it's nbd.

[awestlake87]

I think the implementation is done. I've updated the docs and tested all possible combinations of features with cargo hack test --feature-powerset.

10 is ready to merge, but I'll give it a little while in case anyone has any feedback. Not sure if this is going in the initial release just yet. If not, then it'll probably have to wait until 0.14 since it's a breaking change.

I thought I'd tag @ChillFish8 and @davidhewitt specifically to see if they have any feedback on it. Idk if you guys have made use of the testing utilities yet, but this PR makes a big difference for ergonomics in my project's integration tests.

[kngwyu]

The Test struct has to be a member of the crate where the test functions are defined (a restriction of the inventory crate),

I think inventory allows to do so by the following syntax:

        inventory::submit! {
            #![crate = your_crate] {
               ....
            }
        }

.

See https://github.com/PyO3/pyo3/blob/v0.13.1/pyo3-macros-backend/src/pyproto.rs#L124 for example.

[awestlake87]

Thanks for pointing that out! Knowing that you guys use inventory helped me simplify this a bit.

Re-exporting inventory in pyo3-asyncio and the #![crate = pyo3_asyncio] in the submit! macros means that downstream crates no longer need to depend on inventory for their tests, so that additional caveat no longer exists.

Also having the Test structure in pyo3-asyncio instead of the downstream crate means that the test_structs! macro is no longer needed and test_main_body! can become a function instead of a macro. For me, this calls into question the need for the test_main! macro at all. How much boilerplate are we really saving users when this:

pyo3_asyncio::testing::test_main!(#[pyo3_asyncio::tokio::main], "Example Test Suite");

Just expands to this:

#[pyo3_asyncio::tokio::main]
async fn main() -> pyo3::PyResult<()> {
    pyo3_asyncio::testing::main("Example Test Suite").await
}

I'm kinda leaning towards dropping the test_main! too for this reason.

[davidhewitt]

Sorry it's taken me so long to get around to reading this. It looks like a nice and sensible design to me 👍

I think dropping test_main! is a good idea, as it doesn't reduce much boilerplate now and hides what's actually going on a bit imo.

I wonder whether the argument to the main("Example Test Suite") is needed? If you combine #[track_caller] with std::panic::Location::caller() you may be able to automatically create a user-friendly name for the test suite.

(That requires Rust 1.46+, which is marginally newer than what pyo3 requires. Probably fine...)

[awestlake87]

I can go either way on it personally. I just checked a normal cargo test and it doesn't seem to even list a suite name in the usage string, so we can just drop it for now and worry about improving it later (if it even comes up again).

[davidhewitt]

👍 I think that's probably best. Less is more (and we can always add back stuff later if users ask for it)!

[awestlake87]

Just removed test_main! and the suite_name argument, then merged.

[dmgolembiowski]

@awestlake87 your work on this binding is deeply exciting. I'm eager to see how this turns out. Regarding your earlier woe for macro usage in test cases, does something like this work for you:

---

/Cargo.toml

[package]
name = "package"
version = "0.1.0"
authors = ["author details"]
edition = "2018"
autotests = false
 . . . . .

[[test]]
name = "tests"
path = "src/tests/base.rs"

[dev-dependencies]
trybuild = { version = "1.0", features = ["diff"] }
test-case = { version = "1.0.0" }

---

/src/lib.rs

pub mod x
pub mod y

#[cfg(test)]
extern crate test_case;

---

/src/tests/01-basic-event-loop.rs

You could have something like

use test_case::test_case;

#[test_case(
    /* var_a  = */ "shared_unit__then", 
    /* var_b = */  "callable_test_name_as",
    /* var_c  = */ "build_inductive_case_idk",
    /* var_d  = */ "Thing"; 
    /* descr  = */ "EventLoopPolicy without suspended callbacks")]
fn testcase_wrapper(...)  {
    //   test logical assertions/specs here
}

// ... things

// This is merely a dummy to verify that the dynamic dispatch
// for the test harness works properly.
#[allow(dead_code)]
pub struct Thing {
    declarations: Vec<i32>
}

#[allow(dead_code)]
fn build_inductive_case_idk(var_a: &str) -> Thing {
    Thing { declarations: vec![1] }
}

#[allow(dead_code)]
fn main() {}

Admittedly, I'm naive in this area but I found this pattern helpful before. I'm hoping it can be adapted to fit adjacent to the wasi proc macro.

[dmgolembiowski]

By the by, what all do you need to get this working? I've been wanting to do this exact thing for a long time now but haven't wanted to deal with the enormous complexity.

[awestlake87]

We were actually able to get automatic test discovery working by using the inventory crate in the test attributes to generate a list of tests at compile time. Aside from the boilerplate of the cargo [[test]] entry and the main fn, our #[test] macros work pretty much like the original #[test] macro in integration tests and doc tests (unfortunately not libtests though). You can read more about the testing utilities here. Let me know if you have any questions or if you think anything's missing from the explanation there! I want the docs to be pretty high quality.

By the by, what all do you need to get this working?

The solution we came up with is simple, but it works pretty well. We just use asyncio.Future and futures::channel::oneshot::Sender to communicate the results between the Python and Rust event loops. The tricky part is managing the Python event loop because of the GIL and its reliance on the main thread.

We just merged an entry into the guide (although it's not released yet) that should be able to give you a good quickstart on your projects if you want to check it out. The only caveat is that you should patch your pyo3 to the master branch until 0.13.2 is released (looks like it could be just a few days before that happens).

Edit: As an aside, unfortunately I don't know if the test-case crate will work with our test attributes because we don't actually use the default #[test] attribute in the expansion. This could be problematic, but I'd be curious to see if it works in case you want to try it out. If not, it might be worth supporting one way or another.

[dmgolembiowski]

We were actually able to get automatic test discovery working by using the inventory crate in the test attributes to generate a list of tests at compile time. Aside from the boilerplate of the cargo [[test]] entry and the main fn, our #[test] macros work pretty much like the original #[test] macro in integration tests and doc tests (unfortunately not libtests though). You can read more about the testing utilities here. Let me know if you have any questions or if you think anything's missing from the explanation there! I want the docs to be pretty high quality.

By the by, what all do you need to get this working?

The solution we came up with is simple, but it works pretty well. We just use asyncio.Future and futures::channel::oneshot::Sender to communicate the results between the Python and Rust event loops. The tricky part is managing the Python event loop because of the GIL and its reliance on the main thread.

We just merged an entry into the guide (although it's not released yet) that should be able to give you a good quickstart on your projects if you want to check it out. The only caveat is that you should patch your pyo3 to the master branch until 0.13.2 is released (looks like it could be just a few days before that happens).

Edit: As an aside, unfortunately I don't know if the test-case crate will work with our test attributes because we don't actually use the default #[test] attribute in the expansion. This could be problematic, but I'd be curious to see if it works in case you want to try it out. Thank you for being so welcome!

Sweeet! :smile: I can't wait to sink my teeth into this. I started this compiler-based ORM/FRM framework for EdgeDB project last year and it's been daunting to tackle asyncio compatibility. It's going to be really exciting to start peeling back the layers.

[awestlake87]

Awesome! Let me know if you find any issues or have any suggestions for the library. I've used it in some of my own projects and havent had any problems so far, but it's by no means battle-tested.

Just FYI, the tokio runtime is the one ive tested most so while async-std is supported, it may have problems we dont know about yet.

[dmgolembiowski]

Awesome! Let me know if you find any issues or have any suggestions for the library. I've used it in some of my own projects and havent had any problems so far, but it's by no means battle-tested.

Just FYI, the tokio runtime is the one ive tested most so while async-std is supported, it may have problems we dont know about yet.

Will do!