Fix wasm-issue#702 add feature flag #6726
Conversation
bogdan-stacks
changed the title
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (29.56%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## feat/clarity-wasm-develop #6726 +/- ##
=============================================================
- Coverage 30.91% 29.56% -1.35%
=============================================================
Files 572 572
Lines 358768 358775 +7
=============================================================
- Hits 110903 106083 -4820
- Misses 247865 252692 +4827
... and 119 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| if let Error::ClarityError(clarity_error::Wasm(WasmError::WasmGeneratorError(_))) = &err | ||
| { | ||
| // WASM error type - expected in WASM context | ||
| } else { | ||
| panic!("Did not get unchecked interpreter error or WASM generator error"); | ||
| } |
There was a problem hiding this comment.
This would certainly be cleaner using a matches!.
| #[cfg(feature = "clarity-wasm")] | ||
| if let Error::ClarityError(clarity_error::Wasm(WasmError::WasmGeneratorError(_))) = &err | ||
| { | ||
| // WASM error type - expected in WASM context | ||
| } else { | ||
| panic!("Did not get unchecked interpreter error or WASM generator error"); | ||
| } | ||
| #[cfg(not(feature = "clarity-wasm"))] | ||
| { | ||
| panic!("Did not get unchecked interpreter error"); | ||
| } |
There was a problem hiding this comment.
Using the cfg! macro would certainly make the distinction cleaner for the future readers.
| #[cfg(feature = "clarity-wasm")] | ||
| if let Error::ClarityError(clarity_error::Wasm(WasmError::WasmGeneratorError(_))) = &err | ||
| { | ||
| // WASM error type - expected in WASM context | ||
| } else { | ||
| panic!("Did not get unchecked interpreter error or WASM generator error"); | ||
| } | ||
| #[cfg(not(feature = "clarity-wasm"))] | ||
| { | ||
| panic!("Did not get unchecked interpreter error"); | ||
| } |
| if cfg!(feature = "clarity-wasm") { | ||
| assert!(err_str | ||
| .find("TypeError(CallableType(Trait(TraitIdentifier") | ||
| .is_some()); | ||
| } else { | ||
| assert!(err_str | ||
| .find("TypeValueError(OptionalType(CallableType(Trait(TraitIdentifier") | ||
| .is_some()); | ||
| } |
There was a problem hiding this comment.
Since the procedure is identical with both features, how about having a variable that would contain the expected error string, and then call assert!(err_str.find(expected_err).is_some()).
| .find("TypeError(CallableType(Trait(TraitIdentifier") | ||
| .is_some()); |
There was a problem hiding this comment.
str::contains would be more explicit.
| if cfg!(feature = "clarity-wasm") { | ||
| assert_eq!(acct.nonce, 4); | ||
| } else { | ||
| assert_eq!(acct.nonce, 5); | ||
| } |
There was a problem hiding this comment.
Here also, the procedure is equivalent, we can just move the if cfg!(...) {...} else {...} inside the assert_eq!.
| assert!(err_str | ||
| .find("TypeValueError(OptionalType(CallableType(Trait(TraitIdentifier ") | ||
| .is_some()); |
2 participants
This PR adds a
clarity-wasmfeature flag for certain Clarity1 errors as the WASM and interpreter error behaviors differ slightly, even though they represent the same underlying meaning.Closes: stx-labs/clarity-wasm#702