Skip to content

Commit

Permalink
Update book to reflect testFail removal (#1420)
Browse files Browse the repository at this point in the history
  • Loading branch information
grandizzy authored Feb 3, 2025
1 parent 07d14e6 commit 43bfc20
Show file tree
Hide file tree
Showing 13 changed files with 47 additions and 93 deletions.
8 changes: 0 additions & 8 deletions projects/cheatcodes/test/OwnerUpOnly.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,7 @@ contract OwnerUpOnlyTest is Test {
}
// ANCHOR_END: simple_test

// ANCHOR: test_fail
function testFail_IncrementAsNotOwner() public {
vm.prank(address(0));
upOnly.increment();
}
// ANCHOR_END: test_fail

// ANCHOR: test_expectrevert
// Notice that we replaced `testFail` with `test`
function test_RevertWhen_CallerIsNotOwner() public {
vm.expectRevert(Unauthorized.selector);
vm.prank(address(0));
Expand Down
6 changes: 4 additions & 2 deletions projects/writing_tests/test/Basic.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ contract ContractBTest is Test {
}
// ANCHOR_END: testNumberIs42

// ANCHOR: testFailSubtract43
function testFail_Subtract43() public {
// ANCHOR: testRevert_Subtract43
/// forge-config: default.allow_internal_expect_revert = true
function testRevert_Subtract43() public {
vm.expectRevert();
testNumber -= 43;
}
// ANCHOR_END: testFailSubtract43
Expand Down
29 changes: 1 addition & 28 deletions src/forge/cheatcodes.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,7 @@ $ forge test
{{#include ../output/cheatcodes/forge-test-simple:output}}
```

Let's make sure that someone who is definitely not the owner can't increment the count:

```solidity
{{#include ../../projects/cheatcodes/test/OwnerUpOnly.t.sol:contract_prelude}}
// ...
{{#include ../../projects/cheatcodes/test/OwnerUpOnly.t.sol:test_fail}}
}
```

If we run `forge test` now, we will see that all the test pass.

```ignore
$ forge test
{{#include ../output/cheatcodes/forge-test-cheatcodes:output}}
```

The test passed because the `prank` cheatcode changed our identity to the zero address for the next call (`upOnly.increment()`). The test case passed since we used the `testFail` prefix, however, using `testFail` is considered an anti-pattern since it does not tell us anything about *why* `upOnly.increment()` reverted.

If we run the tests again with traces turned on, we can see that we reverted with the correct error message.

```ignore
$ forge test -vvvv --match-test testFail_IncrementAsNotOwner
{{#include ../output/cheatcodes/forge-test-cheatcodes-tracing:output}}
```

To be sure in the future, let's make sure that we reverted because we are not the owner using the `expectRevert` cheatcode:
Let's make sure that someone who is definitely not the owner can't increment the count, by using the `expectRevert` cheatcode:

```solidity
{{#include ../../projects/cheatcodes/test/OwnerUpOnly.t.sol:contract_prelude}}
Expand Down
24 changes: 12 additions & 12 deletions src/forge/gas-function-snapshots.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ $ cat .gas-snapshot
ERC20Test:testApprove() (gas: 31162)
ERC20Test:testBurn() (gas: 59875)
ERC20Test:testFailTransferFromInsufficientAllowance() (gas: 81034)
ERC20Test:testFailTransferFromInsufficientBalance() (gas: 81662)
ERC20Test:testFailTransferInsufficientBalance() (gas: 52882)
ERC20Test:testRevertTransferFromInsufficientAllowance() (gas: 81034)
ERC20Test:testRevertTransferFromInsufficientBalance() (gas: 81662)
ERC20Test:testRevertTransferInsufficientBalance() (gas: 52882)
ERC20Test:testInfiniteApproveTransferFrom() (gas: 90167)
ERC20Test:testMetadata() (gas: 14606)
ERC20Test:testMint() (gas: 53830)
Expand Down Expand Up @@ -60,19 +60,19 @@ $ forge snapshot --diff .gas-snapshot2
Running 10 tests for src/test/ERC20.t.sol:ERC20Test
[PASS] testApprove() (gas: 31162)
[PASS] testBurn() (gas: 59875)
[PASS] testFailTransferFromInsufficientAllowance() (gas: 81034)
[PASS] testFailTransferFromInsufficientBalance() (gas: 81662)
[PASS] testFailTransferInsufficientBalance() (gas: 52882)
[PASS] testRevertTransferFromInsufficientAllowance() (gas: 81034)
[PASS] testRevertTransferFromInsufficientBalance() (gas: 81662)
[PASS] testRevertTransferInsufficientBalance() (gas: 52882)
[PASS] testInfiniteApproveTransferFrom() (gas: 90167)
[PASS] testMetadata() (gas: 14606)
[PASS] testMint() (gas: 53830)
[PASS] testTransfer() (gas: 60473)
[PASS] testTransferFrom() (gas: 84152)
Test result: ok. 10 passed; 0 failed; finished in 2.86ms
testBurn() (gas: 0 (0.000%))
testFailTransferFromInsufficientAllowance() (gas: 0 (0.000%))
testFailTransferFromInsufficientBalance() (gas: 0 (0.000%))
testFailTransferInsufficientBalance() (gas: 0 (0.000%))
testRevertTransferFromInsufficientAllowance() (gas: 0 (0.000%))
testRevertTransferFromInsufficientBalance() (gas: 0 (0.000%))
testRevertTransferInsufficientBalance() (gas: 0 (0.000%))
testInfiniteApproveTransferFrom() (gas: 0 (0.000%))
testMetadata() (gas: 0 (0.000%))
testMint() (gas: 0 (0.000%))
Expand All @@ -93,9 +93,9 @@ $ forge snapshot --check .gas-snapshot2
Running 10 tests for src/test/ERC20.t.sol:ERC20Test
[PASS] testApprove() (gas: 31162)
[PASS] testBurn() (gas: 59875)
[PASS] testFailTransferFromInsufficientAllowance() (gas: 81034)
[PASS] testFailTransferFromInsufficientBalance() (gas: 81662)
[PASS] testFailTransferInsufficientBalance() (gas: 52882)
[PASS] testRevertTransferFromInsufficientAllowance() (gas: 81034)
[PASS] testRevertTransferFromInsufficientBalance() (gas: 81662)
[PASS] testRevertTransferInsufficientBalance() (gas: 52882)
[PASS] testInfiniteApproveTransferFrom() (gas: 90167)
[PASS] testMetadata() (gas: 14606)
[PASS] testMint() (gas: 53830)
Expand Down
6 changes: 1 addition & 5 deletions src/forge/writing-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ Forge uses the following keywords in tests:
```solidity
{{#include ../../projects/writing_tests/test/Basic.t.sol:testNumberIs42}}
```
- `testFail`: The inverse of the `test` prefix - if the function does not revert, the test fails.
```solidity
{{#include ../../projects/writing_tests/test/Basic.t.sol:testFailSubtract43}}
```

A good practice is to use the pattern `test_Revert[If|When]_Condition` in combination with the [`expectRevert`](../cheatcodes/expect-revert.md) cheatcode (cheatcodes are explained in greater detail in the following [section](./cheatcodes.md)). Also, other testing practices can be found in the [Tutorials section](../tutorials/best-practices.md).

Expand All @@ -42,7 +38,7 @@ A good practice is to use the pattern `test_Revert[If|When]_Condition` in combin
> import {stdError} from "forge-std/StdError.sol";
> ```
Now, instead of using `testFail`, you know exactly what reverted and with which error:
In this way you know exactly what reverted and with which error:
```solidity
{{#include ../../projects/writing_tests/test/Basic2.t.sol:testCannotSubtract43}}
Expand Down
9 changes: 5 additions & 4 deletions src/output/cheatcodes/forge-test-cheatcodes-expectrevert

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions src/output/nft_tutorial/forge-test

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/reference/ds-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ Asserts `a` is approximately equal to `b` with delta in absolute value.
##### Example

```solidity
function testFail () external {
function testRevert () external {
uint256 a = 100;
uint256 b = 200;
Expand All @@ -528,7 +528,7 @@ Asserts `a` is approximately equal to `b` with delta in percentage, where `1e18`
##### Example

```solidity
function testFail () external {
function testRevert () external {
uint256 a = 100;
uint256 b = 200;
assertApproxEqRel(a, b, 0.4e18);
Expand Down
12 changes: 3 additions & 9 deletions src/reference/forge-std/assertApproxEqAbs.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Optionally includes an error message in the revert string.
### Examples

```solidity
function testFail() external {
function testRevert() external {
uint256 a = 100;
uint256 b = 200;
Expand All @@ -36,16 +36,10 @@ function testFail() external {
```

```ignore
[PASS] testFail() (gas: 23169)
Logs:
Error: a ~= b not satisfied [uint]
Expected: 200
Actual: 100
Max Delta: 90
Delta: 100
[FAIL: assertion failed: 100 !~= 200 (max delta: 90, real delta: 100)] testRevert() (gas: 23884)
```

### SEE ALSO

- [`assertApproxEqAbsDecimal`](./assertApproxEqAbsDecimal.md)
- [`assertApproxEqRel`](./assertApproxEqRel.md)
- [`assertApproxEqRel`](./assertApproxEqRel.md)
12 changes: 3 additions & 9 deletions src/reference/forge-std/assertApproxEqRel.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,18 @@ Optionally includes an error message in the revert string.
### Examples

```solidity
function testFail () external {
function testRevert() external {
uint256 a = 100;
uint256 b = 200;
assertApproxEqRel(a, b, 0.4e18);
}
```

```ignore
[PASS] testFail() (gas: 23884)
Logs:
Error: a ~= b not satisfied [uint]
Expected: 200
Actual: 100
Max % Delta: 0.400000000000000000
% Delta: 0.500000000000000000
[FAIL: assertion failed: 100 !~= 200 (max delta: 40.0000000000000000%, real delta: 50.0000000000000000%)] testRevert() (gas: 23884)
```

### SEE ALSO

- [`assertApproxEqRelDecimal`](./assertApproxEqRelDecimal.md)
- [`assertApproxEqAbs`](./assertApproxEqAbs.md)
- [`assertApproxEqAbs`](./assertApproxEqAbs.md)
6 changes: 3 additions & 3 deletions src/reference/forge/forge-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,17 @@ You can pass `--json` to make it easier for outside extensions to parse structur
forge test --gas-report
```

4. Only run tests in `test/Contract.t.sol` in the `BigTest` contract that start with `testFail`:
4. Only run tests in `test/Contract.t.sol` in the `BigTest` contract that start with `testRevert`:
```sh
forge test --match-path test/Contract.t.sol --match-contract BigTest \
--match-test "testFail*"
--match-test "testRevert*"
```

5. List tests in desired format
```sh
forge test --list
forge test --list --json
forge test --list --json --match-test "testFail*" | tail -n 1 | json_pp
forge test --list --json --match-test "testRevert*" | tail -n 1 | json_pp
```

### SEE ALSO
Expand Down
2 changes: 1 addition & 1 deletion src/tutorials/solmate-nft.md
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ The test suite is set up as a contract with a `setUp` method which runs before e

As you can see, Forge offers a number of [cheatcodes](../cheatcodes/) to manipulate state to accommodate your testing scenario.

For example, our `testFailMaxSupplyReached` test checks that an attempt to mint fails when the max supply of NFT is reached. Thus, the `currentTokenId` of the NFT contract needs to be set to the max supply by using the store cheatcode which allows you to write data to your contracts storage slots. The storage slots you wish to write to can easily be found using the
For example, our `test_RevertMintMaxSupplyReached` test checks that an attempt to mint fails when the max supply of NFT is reached. Thus, the `currentTokenId` of the NFT contract needs to be set to the max supply by using the store cheatcode which allows you to write data to your contracts storage slots. The storage slots you wish to write to can easily be found using the
[`forge-std`](https://github.com/foundry-rs/forge-std/) helper library. You can run the test with the following command:

```bash
Expand Down
6 changes: 4 additions & 2 deletions src/tutorials/testing-eip712.md
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ contract ERC20Test is Test {
- Ensure failure for calls with an invalid allowance and invalid balance

```solidity
function testFail_InvalidAllowance() public {
function testRevert_InvalidAllowance() public {
SigUtils.Permit memory permit = SigUtils.Permit({
owner: owner,
spender: spender,
Expand All @@ -431,10 +431,11 @@ contract ERC20Test is Test {
);
vm.prank(spender);
vm.expectRevert();
token.transferFrom(owner, spender, 1e18); // attempt to transfer 1 token
}
function testFail_InvalidBalance() public {
function testRevert_InvalidBalance() public {
SigUtils.Permit memory permit = SigUtils.Permit({
owner: owner,
spender: spender,
Expand All @@ -458,6 +459,7 @@ contract ERC20Test is Test {
);
vm.prank(spender);
vm.expectRevert();
token.transferFrom(owner, spender, 2e18); // attempt to transfer 2 tokens (owner only owns 1)
}
```
Expand Down

0 comments on commit 43bfc20

Please sign in to comment.