refactor: replace callback-results example sim tests with workspaces #694
Conversation
| let res = contract | ||
| .call(&worker, "call_all") | ||
| .args_json((false, 1u8))? | ||
| .gas(300_000_000_000_000) |
There was a problem hiding this comment.
Do we not have a constant for the max prepaid gas somewhere?
There was a problem hiding this comment.
not currently, maybe a good decision for some cases, alternative may be to have another function called max_gas or something to avoid having a constant that they have to find and match themselves.
@ChaoticTempest you might have an idea if any of this would be worth it for workspaces :)
There was a problem hiding this comment.
For testing purposes, it does seem that defaulting to the maximal possible gas makes sense. That is, I'd expect this to read as
let res = contract
.call(&worker, "call_all")
.args_json((false, 1u8))?
.transact()
.await?;without specifying gas explicitly, and maybe instead asserting on the amount of gas spend after the call.
There was a problem hiding this comment.
the only negative with defaulting max gas is that this API is agnostic of network, so this might make sense with sandbox or testnet, but could lead to issues if the developer is unaware of this default on mainnet.
Either case, this will affect when we update workspaces, so I don't think this is blocking this PR?
There was a problem hiding this comment.
Def not blocking!
the only negative with defaulting max gas is that this API is agnostic of network, so this might make sense with sandbox or testnet, but could lead to issues if the developer is unaware of this default on mainnet.
Do we actually inted workspaces to be used with mainnet? My gut feeling is that having a single library to be used both as a testing DSL and as the primary way to do stuff in prod is not great, as the incentives for the two use-cases are rather opposing. I'd probably expect us to:
- start with official low-level RPC-style API which is network-agnostic (this I think what
near-json-rpc-clientis)? This thing needs to be robust and complete, but can be ugly and verbose to use - add official testing DSL which is nice to use, but not necessary maximally complete or robust
- experiment with unofficial or semi-official "nice" clints for the mainnet.
There was a problem hiding this comment.
That's a very valid point, but the issue is just that with the need for users to interact with mainnet to be able to spoon state or pull any data they can query for into the testing environment, my intuition is that we might not want to be too restrictive on what they can do.
Possible that we might want to disallow sending transactions through mainnet, but given how intentional it has to be to set up a key to do so, not sure it's really that much of a foot gun? @ChaoticTempest maybe you have some input with this
There was a problem hiding this comment.
The thing was that a good amount of users wanted the ability to be able to migrate their workflow from a testing environment to a live/prod environment so that they don't have to redo it all over again. That way they can easily migrate their flow having properly tested it in sandbox and having some confidence that it won't fail. I can see us disabling things or hiding things behind feature gates if things don't align well with testing like sending mainnet transactions though
There was a problem hiding this comment.
A little bit more wordy, but makes so much more sense to me personally now. Excited to see workspaces shaping up, great job @ChaoticTempest!
| let res = contract | ||
| .call(&worker, "call_all") | ||
| .args_json((false, 1u8))? | ||
| .gas(300_000_000_000_000) |
There was a problem hiding this comment.
For testing purposes, it does seem that defaulting to the maximal possible gas makes sense. That is, I'd expect this to read as
let res = contract
.call(&worker, "call_all")
.args_json((false, 1u8))?
.transact()
.await?;without specifying gas explicitly, and maybe instead asserting on the amount of gas spend after the call.
|
|
||
| #[tokio::test] | ||
| async fn workspaces_test() -> anyhow::Result<()> { | ||
| let wasm = fs::read("res/callback_results.wasm").await?; |
There was a problem hiding this comment.
Long term, we should figure out some reasonable pattern to get compiled wasm by either recursively invoking cargo or by using artifact deps rust-lang/cargo#9096
There was a problem hiding this comment.
Related near/near-workspaces-rs#54
|
I have just noticed that callback-results example is not being tested by either |
Well, it is already tested through buildkite. Not really sure why there is this overlap tbh. Might be worth in the future removing tests from buildkite and just doing those through github actions |
Yeah, that was my follow-up question :) |
on path to deprecate sim tests we need to replace all usages in examples
Related to #563 #662