Null instead of undefined in TestResult

[agostbiro]

Steps to reproduce

1. Check out the fix/null-instead-of-undefined branch
2. Navigate to js/integration-tests/solidity-tests in the repo
3. Run pnpm i
4. Run pnpm test

Expected result

All tests pass.

Actual result

Many tests fail with the following message:

Error: Expected reason to be undefined for test that didn't fail, instead it is: 'null'

This is caused by returning null instead of undefined as the optional failure reason for succeeding tests.

[agostbiro]

I've tried to repro this in isolation, but no luck.

Rust code: https://github.com/agostbiro/napi-rs-null-vs-undefined/blob/a3b4ed07aa6ab062e003d96ddd2bd0bcd62f6823/src/lib.rs#L15
JS test: https://github.com/agostbiro/napi-rs-null-vs-undefined/blob/a3b4ed07aa6ab062e003d96ddd2bd0bcd62f6823/__test__/index.spec.ts

To execute the JS test, clone the linked repo and then: yarn && yarn test

[AmosAidoo]

I've tried to repro this in isolation, but no luck.

@agostbiro when you say "no luck", are you pointing to the fact that it passes in isolation but fails in the solidity tests?

[AmosAidoo]

Looks like it would always be null if #[napi] is used. #[napi(object)] behaves like we want just like in your isolated test, but unfortunately, the impls will not work with #[napi(object)].
Based on https://napi.rs/docs/concepts/values#undefined, I played around with this idea:

pub struct TestResult {
  // snip
  #[napi(readonly)]
  pub reason: Option<Either<String, ()>>
  // snip
}

Then for the impl From part, I updated reason to:

reason: test_result.reason.map_or(Some(Either::B(())), |reason| Some(Either::A(reason)))

And the tests pass.

[agostbiro]

I've tried to repro this in isolation, but no luck.

@agostbiro when you say "no luck", are you pointing to the fact that it passes in isolation but fails in the solidity tests?

Hey, yeah I mean in the repor it passes, but in Solidity tests we sometimes get null instead of undefined.

[agostbiro]

Looks like it would always be null if #[napi] is used. #[napi(object)] behaves like we want just like in your isolated test, but unfortunately, the impls will not work with #[napi(object)]. Based on https://napi.rs/docs/concepts/values#undefined, I played around with this idea:

pub struct TestResult {
// snip
#[napi(readonly)]
pub reason: Option<Either<String, ()>>
// snip
}
Then for the impl From part, I updated reason to:

reason: test_result.reason.map_or(Some(Either::B(())), |reason| Some(Either::A(reason)))
And the tests pass.

Oh nice, could you open a PR with this?

[AmosAidoo]

Oh nice, could you open a PR with this?

Yes sure