Fix libwasmvm tests for windows and check it in CI
#338
Conversation
| assert!(cache_ptr.is_null()); | ||
| assert!(error_msg.is_some()); | ||
| let msg = String::from_utf8(error_msg.consume().unwrap()).unwrap(); | ||
| assert_eq!(msg, "Error calling the VM: Cache error: Error creating directory broken\u{0}dir/state: data provided contains a nul byte"); |
There was a problem hiding this comment.
this error message isn't committed anywhere in the CW state, right?
There was a problem hiding this comment.
it's potentially also for third-party uses... for example, there was that issue in ibc-go where it committed the error message from Cosmos SDK and the message changed in Cosmos SDK seemingly non-breaking patches (within Cosmos SDK, it wasn't being committed to state anywhere).
but not sure whether wasmvm or wasmd would be used in a similar manner -- @webmaster128 @ethanfrey
There was a problem hiding this comment.
This error only happens when initializing cache. In wasmd, init_cache function will get called here during keeper creation. So, I don't think it gets committed to CW state anywhere.
There was a problem hiding this comment.
App will panic if this error happens.
There was a problem hiding this comment.
ok, that looks all right then -- I guess it's the similar case with https://github.com/CosmWasm/cosmwasm/pull/1369/files/a2c262d7b205e6aab448e24c790800f07ace2234#diff-a33cf9c4a456c8ddde19ce339a98e000023ad1dbdb34ade052cb401950dcc5daR537
| elements_memory_cache: 0, | ||
| size_pinned_memory_cache: 5665691, | ||
| size_memory_cache: 0, | ||
| } |
There was a problem hiding this comment.
@uint are you aware of an implementation similar to this assert_approx_eq but with a relative epsilon? I don't want to say 5665691 +/- 665691 but 5665691 +/- 15%. We could use that for some gas tests as well.
There was a problem hiding this comment.
Jakub or Bart (I don't remember) actually did that for Isotonic. It's easy to implement. I'd just write our own macro in cosmwasm and maybe open a PR to the assert_approx_eq crate.
There was a problem hiding this comment.
That would be much appreciated!
There was a problem hiding this comment.
This particular issue should be solved as part of #351
|
This is superseded in #360 using a different approach to some of the changes here. However, this work has been very helpful to bring up the challenges and solve them in small steps. Thanks a lot @tomtau and @devashishdxt. Full Windows .dll support upstream can be expected as part of the 1.2.0 release. |
4 participants
No description provided.