Willyham Github contribution chart
Willyham Github Stats
Willyham Most Used Languages

Activity

28 Sep 2022

Issue Comment

Willyham

mocking call with no return value does not work

Hey. I can't find a way to mock a call which doesn't return a value. It may be that I am just passing the wrong thing to mockCall, but I can't find any documentation about what the right thing to pass is. Calls with a return value work as expected.

I have tried passing abi.encode(), "", and an empty variable initialised with bytes memory x;.

Reproduction case:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.10;

import {console2 as console} from "forge-std/console2.sol";

interface IFooer {
    function foo() external;

    function fooWithParam(uint256) external;

    function fooWithReturn() external returns (uint256);

    function fooWithParamAndReturn(uint256) external returns (uint256);
}

contract Contract {
    IFooer public fooer;

    constructor(address fooAddr) {
        fooer = IFooer(fooAddr);
    }

    function foo() public {
        fooer.foo();
    }

    function fooWithParam() public {
        fooer.fooWithParam(1);
    }

    function fooWithReturn() public {
        uint256 result = fooer.fooWithReturn();
        console.log(result);
    }

    function fooWithParamAndReturn() public {
        uint256 result = fooer.fooWithParamAndReturn(1);
        console.log(result);
    }
} 

Test:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.10;

import "forge-std/Test.sol";
// import {Vm} from "forge-std/Vm.sol";
import {IFooer, Contract} from "../src/Contract.sol";

contract ContractTest is Test {
    Contract public c;
    address mockAddr = 0xa8C254850E5F54393c8a99D3bc32d759c596D648;

    function setUp() public {
        c = new Contract(mockAddr);
    }

    function testFoo() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.foo.selector),
            abi.encode()
        );
        c.foo();
    }

    function testFooWithParam() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.fooWithParam.selector, 1),
            abi.encode()
        );
        c.fooWithParam();
    }

    function testFooWithReturn() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.fooWithReturn.selector),
            abi.encode(123)
        );
        c.fooWithReturn();
    }

    function testFooWithParamAndReturn() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.fooWithParamAndReturn.selector, 1),
            abi.encode(123)
        );
        c.fooWithParamAndReturn();
    }
} 

Result:

Running 4 tests for test/Contract.t.sol:ContractTest
[FAIL. Reason: Revert] testFoo() (gas: 15869)
[FAIL. Reason: Revert] testFooWithParam() (gas: 16076)
[PASS] testFooWithParamAndReturn() (gas: 19152)
[PASS] testFooWithReturn() (gas: 18945)
Test result: FAILED. 2 passed; 2 failed; finished in 446.21µs 

As an aside, it would be good to include a failure reason in this case, as tracking this down in a complex contract was more difficult. Running at max verbosity has no info in the trace about the mock function being invoked:

Running 4 tests for test/Contract.t.sol:ContractTest
[FAIL. Reason: Revert] testFoo() (gas: 15869)
Traces:
  [223698] ContractTest::setUp() 
    ├─ [166939] → new Contract@0xce71065d4017f316ec606fe4422e11eb2c47c246
    │   └─ ← 722 bytes of code
    └─ ← ()

  [15869] ContractTest::testFoo() 
    ├─ [0] VM::mockCall(0xa8c254850e5f54393c8a99d3bc32d759c596d648, 0xc2985578, 0x) 
    │   └─ ← ()
    ├─ [5021] Contract::foo() 
    │   └─ ← ()
    └─ ← () 

Forked On 21 Sep 2022 at 09:46:47

Willyham

Yes, you need to upgrade forge and then passing abi.encode() as the last param should work. Ideally there would be an override for mockCall which had no data param though.

Either way, it should definitely be in the docs.

Commented On 21 Sep 2022 at 09:46:47
Issue Comment

Willyham

Include decoded logs in `forge test --json`

Component

Forge

Describe the feature you would like

forge test --json currently includes raw logging information in its output, e.g:

 "logs": [
          {
            "address": "0x0000000000000000000000000000000000000000",
            "topics": [
              "0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"
            ],
            "data": "0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000033132330000000000000000000000000000000000000000000000000000000000"
          }
        ], 

It would be nice if it could also include the decoded/interpreted logs, as commands consuming this json are not likely to have the ability to decode these logs.

Additional context

This is to support parsing test output information for inclusion in an editor plugin which allows forge tests to be run continuously when developing.

Forked On 15 Sep 2022 at 06:58:50

Willyham

We want the same log data which is printed by the CLI. In the above case the logs were generated by calling console.log from solidity. We need to turn the encoded data into a UTF-8 string (I'm assuming it's utf8 encoded).

Example of print from the CLI:

[PASS] testComplexFunc() (gas: 8602)
Logs:
  This is a log 

The string "this is a log" should be included for this test in the json output. Note that there may but multiple logs per test, and each should be distinct (so we probably want an array of decoded logs)

Commented On 15 Sep 2022 at 06:58:50
Issue Comment

Willyham

Include decoded logs in `forge test --json`

Component

Forge

Describe the feature you would like

forge test --json currently includes raw logging information in its output, e.g:

 "logs": [
          {
            "address": "0x0000000000000000000000000000000000000000",
            "topics": [
              "0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"
            ],
            "data": "0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000033132330000000000000000000000000000000000000000000000000000000000"
          }
        ], 

It would be nice if it could also include the decoded/interpreted logs, as commands consuming this json are not likely to have the ability to decode these logs.

Additional context

This is to support parsing test output information for inclusion in an editor plugin which allows forge tests to be run continuously when developing.

Forked On 15 Sep 2022 at 05:05:42

Willyham

Existing format looks like this:

{
  "test/Contract.t.sol:ContractTest": {
    "duration": {
      "secs": 0,
      "nanos": 287125
    },
    "test_results": {
      "testComplexFunc()": {
        "success": true,
        "reason": null,
        "counterexample": null,
        "logs": [
          {
            "address": "0x0000000000000000000000000000000000000000",
            "topics": [
              "0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"
            ],
            "data": "0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000011546869732069732061206c6f6720313030000000000000000000000000000000"
          }
        ],
        "kind": {
          "Standard": 8602
        },
        "traces": [],
        "labeled_addresses": {}
      },
      "testComplexFuncAgain()": {
        "success": true,
        "reason": null,
        "counterexample": null,
        "logs": [
          {
            "address": "0x0000000000000000000000000000000000000000",
            "topics": [
              "0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"
            ],
            "data": "0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000012546869732069732061206c6f6720313030300000000000000000000000000000"
          }
        ],
        "kind": {
          "Standard": 8636
        },
        "traces": [],
        "labeled_addresses": {}
      }
    },
    "warnings": []
  }
} 

Maybe a decoded (or should it be deserialized?) property within the logs array?

Commented On 15 Sep 2022 at 05:05:42
Issue Comment

Willyham

Include decoded logs in `forge test --json`

Component

Forge

Describe the feature you would like

forge test --json currently includes raw logging information in its output, e.g:

 "logs": [
          {
            "address": "0x0000000000000000000000000000000000000000",
            "topics": [
              "0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"
            ],
            "data": "0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000033132330000000000000000000000000000000000000000000000000000000000"
          }
        ], 

It would be nice if it could also include the decoded/interpreted logs, as commands consuming this json are not likely to have the ability to decode these logs.

Additional context

This is to support parsing test output information for inclusion in an editor plugin which allows forge tests to be run continuously when developing.

Forked On 15 Sep 2022 at 03:33:27

Willyham

Good question, I hadn't really thought of that. Possibly it should just be an extra field of logLines or decodedLogs or something in the existing output?

The existing non-json command simply prints the logs for each test, so I don't think there's any more structured data available that we'd want anyway?

Commented On 15 Sep 2022 at 03:33:27
Issue Comment

Willyham

Include decoded logs in `forge test --json`

Component

Forge

Describe the feature you would like

forge test --json currently includes raw logging information in its output, e.g:

 "logs": [
          {
            "address": "0x0000000000000000000000000000000000000000",
            "topics": [
              "0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"
            ],
            "data": "0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000033132330000000000000000000000000000000000000000000000000000000000"
          }
        ], 

It would be nice if it could also include the decoded/interpreted logs, as commands consuming this json are not likely to have the ability to decode these logs.

Additional context

This is to support parsing test output information for inclusion in an editor plugin which allows forge tests to be run continuously when developing.

Forked On 04 Sep 2022 at 03:11:02

Willyham

Is this feasible, and if so, does the forge team agree that it would be useful? Trying to figure out if I should go down another path for testing results rather than using the json :)

Commented On 04 Sep 2022 at 03:11:02

Willyham

Fix issue with counting hits

Pushed On 02 Sep 2022 at 05:31:44
Create Branch
Willyham In Willyham/vscode-solidity Create Branchwill-save

Willyham

Visual Studio Code language support extension for Solidity smart contracts in Ethereum https://marketplace.visualstudio.com/items?itemName=JuanBlanco.solidity

On 02 Sep 2022 at 05:23:41

Willyham

Visual Studio Code language support extension for Solidity smart contracts in Ethereum https://marketplace.visualstudio.com/items?itemName=JuanBlanco.solidity

Forked On 02 Sep 2022 at 05:23:02
Issue Comment

Willyham

`forge test --json` writes non-json to stdout

Component

Forge

Have you ensured that all of these are up to date?

  • [X] Foundry
  • [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (f8fe940 2022-08-28T00:04:47.679845Z)

What command(s) is the bug in?

forge test --json

Operating System

macOS (Apple Silicon)

Describe the bug

forge test --json writes some compiler and failing test information to stdout. This makes it more difficult to correctly parse the output of this command. E.g. it can't be piped to JQ or to test result parsers, or consumed by other programs easily.

➜  test-mock git:(main) ✗ forge test --json
[⠢] Compiling...
No files changed, compilation skipped
{"test/Contract.t.sol:ContractTest":{"duration":{"secs":0,"nanos":4885041},"test_results":{"testFoo()":{"success":true,"reason":null,"counterexample":null,"logs":[],"kind":{"Standard":13951},"traces":[],"labeled_addresses":{}},"testFooWithParam()":{"success":true,"reason":null,"counterexample":null,"logs":[],"kind":{"Standard":13717},"traces":[],"labeled_addresses":{}},"testFooWithParamAndReturn()":{"success":false,"reason":"EvmError: Revert","counterexample":null,"logs":[],"kind":{"Standard":13787},"traces":[],"labeled_addresses":{}},"testFooWithReturn()":{"success":true,"reason":null,"counterexample":null,"logs":[{"address":"0x0000000000000000000000000000000000000000","topics":["0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"],"data":"0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000033132330000000000000000000000000000000000000000000000000000000000"}],"kind":{"Standard":16445},"traces":[],"labeled_addresses":{}}},"warnings":[]}}

Failing tests:
Encountered 1 failing test in test/Contract.t.sol:ContractTest
[FAIL. Reason: EvmError: Revert] testFooWithParamAndReturn() (gas: 13787)

Encountered a total of 1 failing tests, 3 tests succeeded 

Would suggest that when --json is enabled, the only outputs are json formatted 🙏

Forked On 29 Aug 2022 at 04:16:03

Willyham

That also doesn't seem to work, at least for failing tests:

➜  test-mock git:(main) ✗ forge test -vv --json --silent
{"test/Contract.t.sol:ContractTest":{"duration":{"secs":0,"nanos":4040458},"test_results":{"testFoo()":{"success":true,"reason":null,"counterexample":null,"logs":[],"kind":{"Standard":13951},"traces":[],"labeled_addresses":{}},"testFooWithParam()":{"success":true,"reason":null,"counterexample":null,"logs":[],"kind":{"Standard":13717},"traces":[],"labeled_addresses":{}},"testFooWithParamAndReturn()":{"success":false,"reason":"EvmError: Revert","counterexample":null,"logs":[],"kind":{"Standard":13787},"traces":[],"labeled_addresses":{}},"testFooWithReturn()":{"success":true,"reason":null,"counterexample":null,"logs":[{"address":"0x0000000000000000000000000000000000000000","topics":["0x41304facd9323d75b11bcdd609cb38effffdb05710f7caf0e9b16c6d9d709f50"],"data":"0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000033132330000000000000000000000000000000000000000000000000000000000"}],"kind":{"Standard":16445},"traces":[],"labeled_addresses":{}}},"warnings":[]}}

Failing tests:
Encountered 1 failing test in test/Contract.t.sol:ContractTest
[FAIL. Reason: EvmError: Revert] testFooWithParamAndReturn() (gas: 13787)

Encountered a total of 1 failing tests, 3 tests succeeded 

Commented On 29 Aug 2022 at 04:16:03
Issue Comment

Willyham

mocking call with no return value does not work

Hey. I can't find a way to mock a call which doesn't return a value. It may be that I am just passing the wrong thing to mockCall, but I can't find any documentation about what the right thing to pass is. Calls with a return value work as expected.

I have tried passing abi.encode(), "", and an empty variable initialised with bytes memory x;.

Reproduction case:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.10;

import {console2 as console} from "forge-std/console2.sol";

interface IFooer {
    function foo() external;

    function fooWithParam(uint256) external;

    function fooWithReturn() external returns (uint256);

    function fooWithParamAndReturn(uint256) external returns (uint256);
}

contract Contract {
    IFooer public fooer;

    constructor(address fooAddr) {
        fooer = IFooer(fooAddr);
    }

    function foo() public {
        fooer.foo();
    }

    function fooWithParam() public {
        fooer.fooWithParam(1);
    }

    function fooWithReturn() public {
        uint256 result = fooer.fooWithReturn();
        console.log(result);
    }

    function fooWithParamAndReturn() public {
        uint256 result = fooer.fooWithParamAndReturn(1);
        console.log(result);
    }
} 

Test:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.10;

import "forge-std/Test.sol";
// import {Vm} from "forge-std/Vm.sol";
import {IFooer, Contract} from "../src/Contract.sol";

contract ContractTest is Test {
    Contract public c;
    address mockAddr = 0xa8C254850E5F54393c8a99D3bc32d759c596D648;

    function setUp() public {
        c = new Contract(mockAddr);
    }

    function testFoo() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.foo.selector),
            abi.encode()
        );
        c.foo();
    }

    function testFooWithParam() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.fooWithParam.selector, 1),
            abi.encode()
        );
        c.fooWithParam();
    }

    function testFooWithReturn() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.fooWithReturn.selector),
            abi.encode(123)
        );
        c.fooWithReturn();
    }

    function testFooWithParamAndReturn() public {
        vm.mockCall(
            mockAddr,
            abi.encodeWithSelector(IFooer.fooWithParamAndReturn.selector, 1),
            abi.encode(123)
        );
        c.fooWithParamAndReturn();
    }
} 

Result:

Running 4 tests for test/Contract.t.sol:ContractTest
[FAIL. Reason: Revert] testFoo() (gas: 15869)
[FAIL. Reason: Revert] testFooWithParam() (gas: 16076)
[PASS] testFooWithParamAndReturn() (gas: 19152)
[PASS] testFooWithReturn() (gas: 18945)
Test result: FAILED. 2 passed; 2 failed; finished in 446.21µs 

As an aside, it would be good to include a failure reason in this case, as tracking this down in a complex contract was more difficult. Running at max verbosity has no info in the trace about the mock function being invoked:

Running 4 tests for test/Contract.t.sol:ContractTest
[FAIL. Reason: Revert] testFoo() (gas: 15869)
Traces:
  [223698] ContractTest::setUp() 
    ├─ [166939] → new Contract@0xce71065d4017f316ec606fe4422e11eb2c47c246
    │   └─ ← 722 bytes of code
    └─ ← ()

  [15869] ContractTest::testFoo() 
    ├─ [0] VM::mockCall(0xa8c254850e5f54393c8a99d3bc32d759c596d648, 0xc2985578, 0x) 
    │   └─ ← ()
    ├─ [5021] Contract::foo() 
    │   └─ ← ()
    └─ ← () 

Forked On 28 Aug 2022 at 07:53:12

Willyham

Will do. Though it is related to the book in the sense that there is no documentation which describes the intended way to use this mock.

Commented On 28 Aug 2022 at 07:53:12