Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.12.0-beta.2 Causes Warnings while running Tests #1774

Open
theory opened this issue Jul 10, 2024 · 9 comments
Open

v0.12.0-beta.2 Causes Warnings while running Tests #1774

theory opened this issue Jul 10, 2024 · 9 comments

Comments

@theory
Copy link
Contributor

theory commented Jul 10, 2024

See, for example, this build, which outputs a couple of these:

warning: unused import: `std::error::Error`
   --> src/lib.rs:303:9
    |
303 |     use std::error::Error;
    |         ^^^^^^^^^^^^^^^^^
    |
help: if this is a test module, consider adding a `#[cfg(test)]` to the containing module
   --> src/lib.rs:297:1
    |
297 | #[pg_schema]
    | ^^^^^^^^^^^^
    = note: `#[warn(unused_imports)]` on by default
    = note: this warning originates in the attribute macro `pg_schema` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `jsonschema` (lib) generated 1 warning (run `cargo fix --lib -p jsonschema` to apply 1 suggestion)

Note that std::error::Error is used in the module and cargo fix --lib -p jsonschema makes no changes. I'm not sure why it thinks pg_schema is unused.

Do I need to put pgrx tests in a module separate from regular Rust tests, perhaps?

Potentially related: #1631

@theory
Copy link
Contributor Author

theory commented Aug 26, 2024

Sad to see this still in v0.12.1:

warning: unused import: `std::error::Error`
   --> src/lib.rs:303:9
    |
303 |     use std::error::Error;
    |         ^^^^^^^^^^^^^^^^^
    |
help: if this is a test module, consider adding a `#[cfg(test)]` to the containing module
   --> src/lib.rs:297:1
    |
297 | #[pg_schema]
    | ^^^^^^^^^^^^
    = note: `#[warn(unused_imports)]` on by default
    = note: this warning originates in the attribute macro `pg_schema` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `jsonschema` (lib) generated 1 warning (run `cargo fix --lib -p jsonschema` to apply 1 suggestion)

You can replicate it thusly:

git clone https://github.com/tembo-io/pg-jsonschema-boon.git
cd pg-jsonschema-boon
make test

@eeeebbbbrrrr
Copy link
Contributor

eeeebbbbrrrr commented Aug 26, 2024

This is going to be a little confusing, so bear with me.

Your mod tests {} (https://github.com/tembo-io/pg-jsonschema-boon/blob/280578f3832d309ea9e95b4778648e5497ad849a/src/lib.rs#L298) uses both rust #[test] functions and pgrx #[pg_test] functions.

It is only the #[test] functions that use std::err::Error. So when cargo-pgrx compiles your extension for its test mode (with the --features pg_test feature turned on), the #[pg_test] functions don't use std::err::Error so you get this warning.

You could split the #[test] functions out into a separate #[cfg(test)] mod {}, and I believe that would take care of this warning. AFAICS, this isn't a pgrx-specific issue.

EDIT:
You could also annotate the use statement like so, and leave everything in the same module:

#[cfg(test)]
use std::err::Error;

@theory
Copy link
Contributor Author

theory commented Aug 26, 2024

Oooh! So weird it didn't happen in v0.11.x… I'll take look, thanks.

@eeeebbbbrrrr
Copy link
Contributor

hmm. I can't answer that. Lots of little bits and bobs have changed between then and now.

@theory
Copy link
Contributor Author

theory commented Aug 26, 2024

Okay, that worked, but it took some fiddling. Looks like #[pg_schema] only works for a module named tests, is that right?

@eeeebbbbrrrr
Copy link
Contributor

Looks like #[pg_schema] only works for a module named tests, is that right?

Hmm. <thinking...>

Yes, I think that's true as the test runner issues sql like SELECT tests.test_name();, so it expects all the tests to be in that schema. Been years since we implemented that, but I think it's because it's basically really hard/impossible to figure out the mod {} name through our test runner framework.

theory added a commit to tembo-io/pg-jsonschema-boon that referenced this issue Aug 26, 2024
Since they compile separately, different dependencies can lead to
warnings (see pgcentralfoundation/pgrx#1774 for details). So move shared
functions to a new `test_util` module, keep the Rust-only tests in `mod
test`, and keep the pgrx tests in `mod tests`. I wish I could put the
pgrx tests in the required `pg_test` schema, but it appears that they
require that the module be named "tests".

Thanks @eeeebbbbrrrr for suggesting this solution.
@theory
Copy link
Contributor Author

theory commented Aug 26, 2024

Fixed in tembo-io/pg-jsonschema-boon#5.

@theory
Copy link
Contributor Author

theory commented Aug 26, 2024

but I think it's because it's basically really hard/impossible to figure out the mod {} name through our test runner framework.

Would it make sense to add a feature parameter with the name? Better to parse it, but perhaps a useable workaround?

@eeeebbbbrrrr
Copy link
Contributor

I don’t even know. I’ve never put any thought into it beyond the hardcoded value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants