Fix extendr_module importing

[CGMossa]

The issue is that these get_<module>_metadata are generated in their respective modules, but then they aren't realy imported in the place where they get recursively called.
Others experienced this, see discord message

[CGMossa]

I accidentally made a branch from a branch where I just was half done doing something.
Please accept the minor documentation edits. I was going to make a full PR of these things.

[multimeric]

Can you write a test for this?

[CGMossa]

I've no idea how to...

[multimeric]

Maybe an integration test that has multiple Rust modules in the same file, each with their own extendr_module?

[CGMossa]

Perfect. I added a test and fixed another extendr_module! issue.

[multimeric]

Hmm it would be nice to be able to make assertions about the generated code. Can you call the metadata function in a test block and assert that it contains all the exported functions, or something?

[CGMossa]

I'll think about that. But no I don't think that this is easily possible.

I'm hoping this is a smal pr that we would get merged soon. And released. It is very annoying to users.

[multimeric]

I understand the desire to get it merged, but from my perspective as a reviewer I have no particular evidence that this works. Each module should define a pub fn #module_metadata_name() -> extendr_api::metadata::Metadata. Are you sure you can't just call that for each module to assert that it exists?

[CGMossa]

First, I did put the integration test that you wanted. And that one shows that this now works.
Second, the way this works, is you have a root module that calls its own meta data functions. Then based on the use statements it calls the other ExtendR modules' meta data.

So these functions are already being called.

I do valid your input and if I can make this more robust in the future, I'll make a PR.

[CGMossa]

This is a an issue with the current code-base that Andy had already warned us about in #404. Quite frankly it is up to people like me to clean this sort of thing up, but I've not done it before now.
So this addresses it part of the way, but generally everywhere we should be able to invoke the macros without
special importing statements.

[CGMossa]

Alright, I think we should merge this ASAP. I'm going to request rewiews now.

[CGMossa]

Oh I see. Anything in `crate/tests` are integration tests. These things are not runtime things, but compile-time issues. I'm not going to add a runtime test for the compile-time proc-macro in this PR.

…

On Thu, Feb 9, 2023 at 4:47 PM Ilia Kosenkov ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In extendr-api/tests/extendr_module_tests.rs <#469 (comment)>: > @@ -0,0 +1,28 @@ +//! Tests for [`extendr_module`] procedural macro. +//! +mod root { I was suggesting putting this (or similar code) into extendrtests -- an R integration test package. There, if you can safely call one of the functions of an inner module, this guarantees we get correct metadata. — Reply to this email directly, view it on GitHub <#469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDVSANUJXPIFXSONMPF6LWWUGRDANCNFSM6AAAAAAUQZTRXU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[CGMossa]

Alright, so the submodule in extendrtests now no longer imports everything, thus it should have made an error just now. It hasn't. So there are now an R integration test as well.