feat(test): unit test external call mock #423

qalisander · 2024-11-28T05:50:40Z

This pr contains corresponding improvements for motsu and two new test cases on_erc721_received and ping_pong_works.
Other test cases are commented. And should be refactored once the final design will be approved.

Resolves OpenZeppelin/stylus-test-helpers#8 OpenZeppelin/stylus-test-helpers#12

PR Checklist

Tests
Documentation
Changelog

…ontract-deployment

netlify · 2024-11-28T05:51:03Z

✅ Deploy Preview for contracts-stylus ready!

Name	Link
🔨 Latest commit	`297b574`
🔍 Latest deploy log	https://app.netlify.com/sites/contracts-stylus/deploys/6769b913bec71a00084f6e63
😎 Deploy Preview	https://deploy-preview-423--contracts-stylus.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

qalisander · 2024-11-28T05:55:59Z

contracts/src/utils/context.rs

+#[cfg(test)]
+pub fn msg_sender() -> Address {
+    motsu::prelude::Context::current()
+        .get_msg_sender()
+        .expect("msg_sender should be set")
+}
+
+/// Returns the address of the message sender.
+#[cfg(not(test))]
+pub fn msg_sender() -> Address {
+    msg::sender()
+}


Here is an example how msg_sender mock can be declared. This design is not perfect since motsu users should implement this mock for their own library manually or through macro.
Calling stylus sdk api msg::sender() won't work inside a test without mock due to this limitation.

qalisander · 2024-11-28T05:57:46Z

contracts/src/token/erc721/mod.rs

+    #[motsu::test]
+    fn on_erc721_received(
+        erc721: Contract<Erc721>,
+        receiver: Contract<Erc721ReceiverMock>,
+    ) {
+        let alice = Account::random();
+        let token_id = random_token_id();
+        erc721
+            .sender(alice)
+            ._safe_mint(receiver.address(), token_id, vec![0, 1, 2, 3].into())
+            .unwrap();
+
+        let received_token_id = receiver.sender(alice).received_token_id();
+
+        assert_eq!(received_token_id, token_id);
+    }


Overall the test case for Erc721Receiver can look like this

…eta.1 # Conflicts: # contracts/src/token/erc20/extensions/permit.rs # contracts/src/utils/math/storage.rs # lib/e2e/src/lib.rs

bidzyyys · 2024-12-10T15:21:24Z

This change should be common with #441

0xNeshi

Imo the API is very good, especially as it conforms to existing conventions for writing code that mocks senders, values etc.

I think the builder pattern is the right choice here, as we might need to extend the functionality at a later point (e.g. to add .with_block_timestamp, but there may be others, who knows).

Good job! 🔥

Once clippy warnings are addressed I'll give this a final review, as the PR is too cluttered with warnings atm

…eta.1 # Conflicts: # Cargo.toml # contracts/src/utils/math/storage.rs

…ontract-deployment # Conflicts: # contracts/src/token/erc721/mod.rs # lib/motsu/src/shims.rs

…ntract-deployment-v0.7.0-beta # Conflicts: # contracts/src/token/erc721/mod.rs

…eta.1 # Conflicts: # contracts/src/access/control.rs # contracts/src/finance/vesting_wallet.rs # contracts/src/token/erc1155/mod.rs # contracts/src/token/erc1155/receiver.rs # contracts/src/token/erc20/extensions/capped.rs # contracts/src/token/erc20/extensions/metadata.rs # contracts/src/token/erc721/extensions/enumerable.rs # contracts/src/token/erc721/mod.rs # contracts/src/utils/metadata.rs # contracts/src/utils/pausable.rs

…ntract-deployment-v0.7.0-beta # Conflicts: # contracts/src/token/erc1155/extensions/uri_storage.rs # contracts/src/token/erc721/mod.rs # lib/motsu-proc/src/test.rs

…to unit-tests/multiple-contract-deployment

…ontract-deployment # Conflicts: # Cargo.lock # contracts/src/finance/vesting_wallet.rs # contracts/src/token/erc1155/mod.rs # lib/motsu-proc/src/test.rs # lib/motsu/Cargo.toml # lib/motsu/src/context.rs # lib/motsu/src/lib.rs # lib/motsu/src/prelude.rs # lib/motsu/src/shims.rs

0xNeshi · 2024-12-24T07:26:36Z

contracts/Cargo.toml

 keccak-const.workspace = true
 openzeppelin-stylus-proc.workspace = true

 [dev-dependencies]
 alloy-primitives = { workspace = true, features = ["arbitrary"] }
 motsu.workspace = true
 rand.workspace = true
+stylus-sdk.workspace = true


stylus-sdk is listed both in dependencies and in dev-dependencies

Since OL agreed to have a default feature for ENABLING hostio caching, now we should write stylus-sdk = { version = "0.7.0", default-features = false } on the workspace level, and stylus-sdk = { workspace = true, default-features = true } for [dependencies]. Because we need this caching for wasm compiled contracts, but can not run motsu tests with it.
And use it without default features for unit tests [dev-dependencies]

I would better add a feature for disabling caching (not enabling). It would be more simple

0xNeshi · 2024-12-24T07:27:37Z

contracts/src/access/control.rs

@@ -41,6 +41,8 @@
 //! accounts that have been granted it. We recommend using
 //! `AccessControlDefaultAdminRules` to enforce additional security measures for
 //! this role.
+use alloc::vec::Vec;


Why are we adding this import?

0xNeshi · 2024-12-24T07:28:45Z

contracts/src/finance/vesting_wallet.rs

-pub use token::IErc20;
-mod token {
-    #![allow(missing_docs)]
-    #![cfg_attr(coverage_nightly, coverage(off))]
-    stylus_sdk::stylus_proc::sol_interface! {
-        /// Interface of the ERC-20 token.
-        interface IErc20 {
-            function balanceOf(address account) external view returns (uint256);
-        }
+sol_interface! {
+    /// Interface of the ERC-20 token.
+    #[allow(missing_docs)]
+    interface IErc20 {
+        function balanceOf(address account) external view returns (uint256);


Won't coverage be reduced by including this?

Yeap. It will be reduced, currently the simplest way to compile it.

It doesn't compile if you do:

pub use token::IErc20; mod token { #![allow(missing_docs)] #![cfg_attr(coverage_nightly, coverage(off))] stylus_sdk::prelude::sol_interface! { /// Interface of the ERC-20 token. interface IErc20 { function balanceOf(address account) external view returns (uint256); } } }

? 😮

IErc20 now made private. So it will de-sugar to struct IErc20, instead of pub struct IErc20. And visibility level will be just module wise

May be try to change it again in stylus sdk.. pr

qalisander added 21 commits November 21, 2024 17:49

add storage guard

64cb1ec

comment non erc721 unit test

e2fafe8

++

02d3468

++

d126f0e

++

ef905e5

add ping pong test case

7883a64

add test router

f49daf5

add call shim

1dc7519

++

092a370

add msg::value

e5028fc

fix deadlock

c03429d

ping pong works

0c5ac77

++

924e72e

++

59cc1a2

++

110d340

++

aa577f2

comment erc721 tests

01401e8

add erc721 receiver test

0cd4299

use contract wrapper in motsu params

2b944ce

add msg sender mock

0aaa81e

Merge remote-tracking branch 'origin/main' into unit-tests/multiple-c…

b920606

…ontract-deployment

qalisander commented Nov 28, 2024

View reviewed changes

qalisander marked this pull request as ready for review November 28, 2024 05:59

qalisander requested review from bidzyyys and 0xNeshi as code owners November 28, 2024 05:59

qalisander added 2 commits November 28, 2024 10:32

++

33454f6

++

299ecc4

0xNeshi mentioned this pull request Nov 29, 2024

feat: Investigate Test Coverage Drop #427

Closed

qalisander added 2 commits December 9, 2024 17:08

update alloy to 0.8.14

eb5cc95

Merge remote-tracking branch 'origin/main' into build/stylus-v0.7.0-b…

378da95

…eta.1 # Conflicts: # contracts/src/token/erc20/extensions/permit.rs # contracts/src/utils/math/storage.rs # lib/e2e/src/lib.rs

0xNeshi reviewed Dec 10, 2024

View reviewed changes

qalisander added 10 commits December 11, 2024 16:17

Merge remote-tracking branch 'origin/main' into build/stylus-v0.7.0-b…

e7d074f

…eta.1 # Conflicts: # Cargo.toml # contracts/src/utils/math/storage.rs

++

c255a90

import alloc::vec::Vec for permit

f8b7645

remove unused code

0f2a246

Merge remote-tracking branch 'origin/main' into unit-tests/multiple-c…

a5d40c1

…ontract-deployment # Conflicts: # contracts/src/token/erc721/mod.rs # lib/motsu/src/shims.rs

Merge branch 'build/stylus-v0.7.0-beta.1' into unit-tests/multiple-co…

843ded7

…ntract-deployment-v0.7.0-beta # Conflicts: # contracts/src/token/erc721/mod.rs

++

f27aed9

++

f0f94ac

test ping_pong_works pass with msg::sender()

32ad8fd

remove context msg_sender

0b31370

0xNeshi mentioned this pull request Dec 13, 2024

[Feature]: Allow setting contract::address in unit tests OpenZeppelin/stylus-test-helpers#5

Open

1 task

qalisander added 6 commits December 13, 2024 19:59

post merge fix

b6898fc

++

a16de97

++

acd843a

Merge branch 'build/stylus-v0.7.0-beta.1' into unit-tests/multiple-co…

47a3aa3

…ntract-deployment-v0.7.0-beta # Conflicts: # contracts/src/token/erc1155/extensions/uri_storage.rs # contracts/src/token/erc721/mod.rs # lib/motsu-proc/src/test.rs

post merge fix

9a84c14

qalisander force-pushed the unit-tests/multiple-contract-deployment branch from 35b1c5f to f27aed9 Compare December 13, 2024 17:01

alloy-primitives 0.8.14 -> 0.8.13

ed41209

qalisander mentioned this pull request Dec 19, 2024

feat: unit test external call mock OpenZeppelin/stylus-test-helpers#14

Draft

3 tasks

qalisander added 4 commits December 23, 2024 22:12

++

780c6ec

Merge branch 'unit-tests/multiple-contract-deployment-v0.7.0-beta' in…

1487ee0

…to unit-tests/multiple-contract-deployment

patch motsu

297b574

0xNeshi reviewed Dec 24, 2024

View reviewed changes

qalisander changed the title ~~feat(test): unit test external call mock POC~~ feat(test): unit test external call mock Dec 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test): unit test external call mock #423

feat(test): unit test external call mock #423

qalisander commented Nov 28, 2024

netlify bot commented Nov 28, 2024 •

edited

Loading

qalisander Nov 28, 2024 •

edited

Loading

qalisander Nov 28, 2024 •

edited

Loading

bidzyyys commented Dec 10, 2024

0xNeshi left a comment •

edited

Loading

0xNeshi Dec 24, 2024

qalisander Dec 24, 2024 •

edited

Loading

qalisander Dec 24, 2024 •

edited

Loading

0xNeshi Dec 24, 2024

0xNeshi Dec 24, 2024

qalisander Dec 24, 2024

0xNeshi Dec 24, 2024

qalisander Dec 24, 2024 •

edited

Loading

qalisander Dec 24, 2024 •

edited

Loading

feat(test): unit test external call mock #423

Are you sure you want to change the base?

feat(test): unit test external call mock #423

Conversation

qalisander commented Nov 28, 2024

PR Checklist

netlify bot commented Nov 28, 2024 • edited Loading

✅ Deploy Preview for contracts-stylus ready!

qalisander Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

qalisander Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

bidzyyys commented Dec 10, 2024

0xNeshi left a comment • edited Loading

Choose a reason for hiding this comment

0xNeshi Dec 24, 2024

Choose a reason for hiding this comment

qalisander Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

qalisander Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

0xNeshi Dec 24, 2024

Choose a reason for hiding this comment

0xNeshi Dec 24, 2024

Choose a reason for hiding this comment

qalisander Dec 24, 2024

Choose a reason for hiding this comment

0xNeshi Dec 24, 2024

Choose a reason for hiding this comment

qalisander Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

qalisander Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

netlify bot commented Nov 28, 2024 •

edited

Loading

qalisander Nov 28, 2024 •

edited

Loading

qalisander Nov 28, 2024 •

edited

Loading

0xNeshi left a comment •

edited

Loading

qalisander Dec 24, 2024 •

edited

Loading

qalisander Dec 24, 2024 •

edited

Loading

qalisander Dec 24, 2024 •

edited

Loading

qalisander Dec 24, 2024 •

edited

Loading