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

Unable to run Rust unittests on binded library #170

Open
ziv opened this issue Aug 10, 2021 · 5 comments
Open

Unable to run Rust unittests on binded library #170

ziv opened this issue Aug 10, 2021 · 5 comments
Labels
bug Something isn't working question Further information is requested

Comments

@ziv
Copy link

ziv commented Aug 10, 2021

Running Rust testing on binded library generate errors.

Reproduction:
https://github.com/ziv/node-bindgen-unitest-error-reproduction

Expected behavior:

Running tests should pass.

Actual behavior:

Error

 %  cargo test --verbose
 
    Compiling node-bindgen-unitest-error-reproduction v0.1.0 (...)
     Running `rustc ...`
error: cannot find attribute `node_bindgen` in this scope
 --> src/lib.rs:6:7
  |
6 |     #[node_bindgen]
  |       ^^^^^^^^^^^^

warning: unused import: `node_bindgen::derive::node_bindgen`
 --> src/lib.rs:1:5
  |
1 | use node_bindgen::derive::node_bindgen;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error: aborting due to previous error; 1 warning emitted

error: could not compile `node-bindgen-unitest-error-reproduction`

Caused by:
  process didn't exit successfully: `rustc --crate-name node_bindgen_unitest_error_reproduction --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 --test -C metadata=51601585b678e8bc -C extra-filename=-51601585b678e8bc --out-dir /Users/ziv.perry/zivcode/node-bindgen-unitest-error-reproduction/target/debug/deps -C incremental=/Users/ziv.perry/zivcode/node-bindgen-unitest-error-reproduction/target/debug/incremental -L dependency=/Users/ziv.perry/zivcode/node-bindgen-unitest-error-reproduction/target/debug/deps --extern node_bindgen=/Users/ziv.perry/zivcode/node-bindgen-unitest-error-reproduction/target/debug/deps/libnode_bindgen-553b1a9a13c1f95a.rlib -C link-arg=-undefined -C link-arg=dynamic_lookup` (exit status: 1)
 

Workaround

Removing #[node_bindgen] decorator works.
Example for output after commented the decorator:

 % cargo test
   Compiling node-bindgen-unitest-error-reproduction v0.1.0 (...)
    Finished test [unoptimized + debuginfo] target(s) in 0.91s
     Running unittests (target/debug/deps/node_bindgen_unitest_error_reproduction-51601585b678e8bc)

running 1 test
test testing::should_sum_numbers ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
@nicholastmosher
Copy link
Contributor

Hi @ziv, thanks for opening the issue, and for the great reproduction! I think I see two things happening here:

  1. You are importing node_bindgen::derive::node_bindgen at the top-level module, but you are trying to use it inside your example module. Try putting your use statement inside of each module where you use #[node_bindgen].

  2. I tried fixing 1 and then saw a new error:

error[E0432]: unresolved import `crate::example::sum`
 --> src/testing.rs:1:5                              
  |
1 | use crate::example::sum;
  |     ^^^^^^^^^^^^^^^^^^^ no `sum` in `example`

error: aborting due to previous error

I think this is due to the fact that node_bindgen actually reads your function body and generates a totally different function from it, which is intended to be called from node rather than Rust. For unit testing, I would suggest making an inner Rust function that does your logic and test against that, then create a wrapper that has the #[node_bindgen] annotation and calls the inner function. Something like this:

#[node_bindgen]
pub fn sum(first: i32, second: i32) -> i32 {
    sum_impl(first, second)
}

pub fn sum_impl(first: i32, second: i32) -> i32 {
    first + second
}

// Then unit test `sum_impl`

Let me know if that pattern works for you!

@sehz
Copy link
Collaborator

sehz commented Aug 14, 2021

Seems like this is solved problem. Closing this. Feel to re-open if this doesn't the solve

@sehz sehz closed this as completed Aug 14, 2021
@sehz sehz added question Further information is requested documentation Improvements or additions to documentation labels Aug 14, 2021
@sehz
Copy link
Collaborator

sehz commented Aug 14, 2021

We should add this to documentation

@ziv
Copy link
Author

ziv commented Aug 15, 2021

Hi @nicholastmosher, while your solution is simple to use when my API is minimal, I have a library exports structs and their implementations.

Using this method is practically - copy all my code just to export it.

@sehz
Copy link
Collaborator

sehz commented Aug 15, 2021

Test failure happens because actual rust fn sum is hidden:

extern "C" fn napi_sum(
    env: node_bindgen::sys::napi_env,
    cb_info: node_bindgen::sys::napi_callback_info,
) -> node_bindgen::sys::napi_value {
   .....
    fn sum(first: i32, second: i32) -> i32 {

So one way is to move inner fn as peer:

 pub fn sum(first: i32, second: i32) -> i32 {
...
}

extern "C" fn napi_sum(
    env: node_bindgen::sys::napi_env,
    cb_info: node_bindgen::sys::napi_callback_info,
) -> node_bindgen::sys::napi_value {
   .....
   

@sehz sehz reopened this Aug 15, 2021
@sehz sehz added bug Something isn't working and removed documentation Improvements or additions to documentation labels Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants