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

wasm32-unknown-unknown unit-tests #467

Open
matklad opened this issue Jul 12, 2021 · 12 comments
Open

wasm32-unknown-unknown unit-tests #467

matklad opened this issue Jul 12, 2021 · 12 comments

Comments

@matklad
Copy link
Contributor

matklad commented Jul 12, 2021

Crazy idea to follow up on sandbox discussion. TL;DR: cargo test --target wasm32-unknown-unknown could just work.

Today, near-sdk-rs contracts support unit-tests (#<span class="error">[test]</span>), but in a very round-about way. To run unit-tests, you need to compile the contract for host. That means that contract code can't assume that it runs under wasm, which is a rather big assumption.

It's simpler to require that contract always compiles to wasm (there's an [upcoming cargo feature](rust-lang/cargo#9406) for this), and just make unit-test working with cargo test --target wasm32-unknown-unknown. Amusingly, that [almost works](https://internals.rust-lang.org/t/running-tests-on-wasm32-unknown-unknown/15010?u=matklad) today.

The main trick here are [custom cargo runners](https://doc.rust-lang.org/cargo/reference/config.html#targettriplerunner) – you can tell cargo "please, run .wasm file using this runner (eg, wasmtime, or standalone-vm-runner)". On the Rust side, cargo test --no-run produces a perfectly normal .wasm file – Rust's test harness is compatible with wasm.

In practiece, I think we should do the following:

  • provide a runner that can run .wasm files with near contract ABI. Seems like the sandbox binary can get the ability to run .wasm directly, without running the chain/network/etc.
  • maybe provide the additional test ABI so that tests can manipulate environment from within wasm.
  • set the cargo.runner config to our runner
  • either work with upstream to teach libtest to custom printing functions (see [irlo thread](https://internals.rust-lang.org/t/running-tests-on-wasm32-unknown-unknown/15010) for details) or otherwise provide #<span class="error">[near_test]</span> macro, which expands to extern "C" __near_test__xxxxx. In the runner, we can then just run all the functions which start from this prefix.
@matklad
Copy link
Contributor Author

matklad commented Jul 13, 2021

Thinking more about it, I think there's a better practical solution here -- use wasm32-wasi as a target tripple for testing. The wasi subset actually used by the tests is pretty small, so supporting that in the sandbox shoulnd't be a problem (and I bet that's already avaialble in wasmer):

  (import "wasi_snapshot_preview1" "args_get" (func $wasi_snapshot_preview1.args_get (type $t10)))
  (import "wasi_snapshot_preview1" "args_sizes_get" (func $wasi_snapshot_preview1.args_sizes_get (type $t10)))
  (import "wasi_snapshot_preview1" "clock_time_get" (func $wasi_snapshot_preview1.clock_time_get (type $t12)))
  (import "wasi_snapshot_preview1" "fd_close" (func $wasi_snapshot_preview1.fd_close (type $t9)))
  (import "wasi_snapshot_preview1" "fd_read" (func $wasi_snapshot_preview1.fd_read (type $t13)))
  (import "wasi_snapshot_preview1" "fd_write" (func $wasi_snapshot_preview1.fd_write (type $t13)))
  (import "wasi_snapshot_preview1" "path_filestat_get" (func $wasi_snapshot_preview1.path_filestat_get (type $t14)))
  (import "wasi_snapshot_preview1" "path_open" (func $wasi_snapshot_preview1.path_open (type $t15)))
  (import "wasi_snapshot_preview1" "sched_yield" (func $wasi_snapshot_preview1.sched_yield (type $t16)))
  (import "wasi_snapshot_preview1" "random_get" (func $wasi_snapshot_preview1.random_get (type $t10)))
  (import "wasi_snapshot_preview1" "fd_prestat_get" (func $wasi_snapshot_preview1.fd_prestat_get (type $t10)))
  (import "wasi_snapshot_preview1" "fd_prestat_dir_name" (func $wasi_snapshot_preview1.fd_prestat_dir_name (type $t11)))
  (import "wasi_snapshot_preview1" "proc_exit" (func $wasi_snapshot_preview1.proc_exit (type $t1)))
  (import "wasi_snapshot_preview1" "environ_sizes_get" (func $wasi_snapshot_preview1.environ_sizes_get (type $t10)))
  (import "wasi_snapshot_preview1" "environ_get" (func $wasi_snapshot_preview1.environ_get (type $t10)))

@ailisp
Copy link
Member

ailisp commented Jul 14, 2021

provide a runner that can run .wasm files with near contract ABI. Seems like the sandbox binary can get the ability to run .wasm directly, without running the chain/network/etc.

One thing to note, this sandbox is at a different level of the near-sandbox, which acts on the RPC level - contracts are deployed, then call both by submit an rpc to a running near-sandbox process. This "sandbox" is close to near-vm-standalone, which, used as a cargo runner you provide contract, function call args via command line arguments. And, it's unit test, cross contract calls are mocked and tested in single step.

Should we just make from near-vm-runner-standalone? Or wrap vm-runner-standalone as a new, "vm only" mode for near-sandbox. The second looks unnecessary to me, but we can do it if make sense on developer experience or maintainability

@matklad
Copy link
Contributor Author

matklad commented Jul 14, 2021

Yeah, it's true that what we need here is something a-la standalone runner. Logistically, it's simpler to ship a single binary to the users, which can both run the wasm contract directly, as well as serve as a full-fledged RPC node.

@matklad
Copy link
Contributor Author

matklad commented Jul 14, 2021

Thinking more about it,

Thinking even more, using wasi just to get the standard test harness working is solving the wrong problem. We don't need standard harness, better if sandbox runs the contract directly. We need to use a custom test macro for that, but overall UX seems much better that way. See https://github.com/matklad/webassembly-test for a proof of concept.

@austinabell
Copy link
Contributor

Just to confirm, the intention of this is to allow unit tests to be run as compiled to wasm32-unknown-unknown, or do you mean by extension running those tests on the sandbox by deploying this modified contract which contains the test methods and calling those methods through RPC?

If the former, is the benefit just so that there are no wasm arch-specific logic bugs?

Potentially what we could do also, to not complicate the sandbox binary, is have a test annotation, say #[near::test] which would wrap the test so that it initializes the runtime (so that the syscalls are setup for wasm arch) and runs the test through this? I'm not quite sure how possible/easy this would be, but would be cool to have built in to the rust test framework instead of requiring a binary to run the tests

@matklad
Copy link
Contributor Author

matklad commented Jul 14, 2021

Just to confirm, the intention of this is to allow unit tests to be run as compiled to wasm32-unknown-unknown,

Yes, the intention is to just run unit-tests compiled to wasm32-unknown-unknown. The benefits here are:

  • we test what we deploy (no wasm arch-specific logic bugs)
  • contracts can assume they are compiled to wasm and use wasm-specific functionality. For example, they can use core::arch::wasm32 and assume that there's only single thread (today, tests can be run in parallel when compiled by host)
  • we'll get to re-use unit-testing infra between different languages. If unit-tests work on wasm32, you can run Rust and AssemblyScript the same way. All the host functions live in the wasm runner, which can be shared by different SDKs.

but would be cool to have built in to the rust test framework instead of requiring a binary to run the tests

I am not sure how that would work -- in the end, you have a .wasm blob, and you need to run it somehow.

@MaksymZavershynskyi
Copy link
Contributor

@mikedotexe , @matklad . To clarify, this seems to be a great idea, but it does not seem to be higher in priority than other items that we planned for Q3 in Contract Runtime and Developer Platform. It also does not supersede sandbox-based testing framework.

@matklad
Copy link
Contributor Author

matklad commented Jul 19, 2021

Yeah, agree that there isn't anything immediately actionable here, sorry for not making this clear in the original description.

What would be useful though it to create some sort of a meta issue which describes the short/medium term roadmap for near contract testing.

I still don't have even a remote understanding of where near-sdk-rs unit-tests, sim tests, and near-sdk-as unit-tests should be half a year from now. That would be a useful input to issues like: near/nearcore#4379.

@petersalomonsen
Copy link

I think this has become even more important if we want to combine Rust and Javascript contracts with QuickJS which is written in C. In this case I have provided static libraries compiled for wasm target, but that does not go very well together with near-sdk-rs unit tests which are compiled for the host.

I'm working on being able to create unit/integration tests here: https://github.com/petersalomonsen/quickjs-rust-near

@austinabell
Copy link
Contributor

related near/nearcore#7220.

The only issues I have with this proposal are that:

  • Requires a sandbox binary (a lot of OS/arch are not supported, including windows)
    • This can change in the future, but still not ideal to use pre-built binaries or require a very large and error-prone compilation
  • This wouldn't be testing the codegen that wraps the functions called by the tests (no change from current unit tests, but a large limitation/footgun of current unit tests)
  • No cross-contract support (again, no change from the current unit tests, but not ideal)

The reason for the request in the issue above is to have a more minimal wrapper that has all of the core execution logic that more closely matches the actual execution on-chain without the large overhead of sandbox. This way, Rust tests could just compile the library, and for JS/other languages, a binary could be created, exposing the interface to be able to execute these.

This definitely feels like it's worth having a high-level discussion created for the high-level and long-term goals of testing cross-language.

wdyt @ChaoticTempest @ailisp

@petersalomonsen
Copy link

I understand there might be some challenges here and issues with the testing tools of near-sdk-rs (which does not support the wasm target), but still I think it's worth to explore how far we can get with testing in wasm. At least it makes it very much easier when you deal with libraries linked from C (like QuickJS). First I tried with the host platform target, but on my Mac M1 this is even more fuzz since QuickJS compiles for ARM by default. I will then have to tweak libraries I link to make sure they compile for x86. So if we can get everything working on one common target (wasm), that would be the easiest from the "user" perspective.

Here's my initial attempt on testing with the wasm target (using normal #[cfg(test)]:

image

@petersalomonsen
Copy link

and here's the first test run in github actions: https://github.com/petersalomonsen/quickjs-rust-near/runs/7886061254?check_suite_focus=true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: NEW❗
Development

No branches or pull requests

5 participants