Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(test): assertApproxEq, assertNotEq, assertFalse #38

Merged
merged 19 commits into from
Apr 22, 2022
Merged

feat(test): assertApproxEq, assertNotEq, assertFalse #38

Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e17d6f0
feat(test): assertApproxEq, assertNotEq, assertFalse
OliverNChalk Apr 18, 2022
1e3b6a2
refactor: yoink assertApproxEq from t11s
OliverNChalk Apr 20, 2022
fb089f0
feat(test): yoink assertRelApproxEq from t11s
OliverNChalk Apr 20, 2022
dfaa2d5
style(test): align multi-line log messages for character alignment
OliverNChalk Apr 20, 2022
7e73c21
fix(test): correct yoink/translation errors
OliverNChalk Apr 20, 2022
1a78d1b
feat(test): add more bool asserts, t11s assertFalse
OliverNChalk Apr 20, 2022
3e8d693
feat(test): add assertEq(bool, err)
OliverNChalk Apr 20, 2022
b61c6c4
feat(test): add assertEq(bytes, bytes)
OliverNChalk Apr 20, 2022
51ff84f
refactor(test): standardized approx eq assertion naming
OliverNChalk Apr 21, 2022
3e0ef00
refactor(test): drop assertNotEq as it's bad practice
OliverNChalk Apr 21, 2022
1f08a76
refactor(test): improve assertEq(bool,bool) error messaging
OliverNChalk Apr 21, 2022
f0e1ba9
fix(test): remove duplicate assertEq(bool,bool)
OliverNChalk Apr 21, 2022
03355c9
fix(test): use log instead of log_named_string
OliverNChalk Apr 21, 2022
2c23374
feat(test): add int256 impls for assertApproxEqRel
OliverNChalk Apr 21, 2022
924456a
style(test): re-order code and group overloads together
OliverNChalk Apr 21, 2022
99a36d8
style(test): use single STD-ASSERTIONS code region
OliverNChalk Apr 22, 2022
3c15901
style(test): declare assertFalse before assertEq
OliverNChalk Apr 22, 2022
113b11c
style(test): add STD-CHEATS code region
OliverNChalk Apr 22, 2022
b434995
style: add STD-ERRORS & STD-STORAGE delimters
OliverNChalk Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 216 additions & 8 deletions src/Test.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,214 @@ abstract contract Test is DSTest {
addr := create(0, add(bytecode, 0x20), mload(bytecode))
}
}

/*//////////////////////////////////////////////////////////////////////////
ASSERT EQ
//////////////////////////////////////////////////////////////////////////*/

function assertEq(bool a, bool b) internal {
if (a != b) {
emit log ("Error: a == b not satisfied [bool]");
emit log_named_string (" Expected", b ? "true" : "false");
emit log_named_string (" Actual", a ? "true" : "false");
fail();
}
}

function assertEq(bool a, bool b, string memory err) internal {
if (a != b) {
emit log_named_string("Error", err);
emit log_named_string(" Expected", b ? "true" : "false");
emit log_named_string(" Actual", a ? "true" : "false");
fail();
}
}

function assertEq(bytes memory a, bytes memory b) internal virtual {
if (keccak256(a) != keccak256(b)) {
emit log ("Error: a == b not satisfied [bytes]");
emit log_named_bytes(" Expected", b);
emit log_named_bytes(" Actual", a);
fail();
}
}

function assertEq(bytes memory a, bytes memory b, string memory err) internal virtual {
if (keccak256(a) != keccak256(b)) {
emit log_named_string ("Error", err);
emit log_named_bytes (" Expected", b);
emit log_named_bytes (" Actual", a);
fail();
}
}

/*//////////////////////////////////////////////////////////////////////////
ASSERT FALSE
//////////////////////////////////////////////////////////////////////////*/

function assertFalse(bool data) internal virtual {
assertTrue(!data);
}

function assertFalse(bool data, string memory err) internal virtual {
assertTrue(!data, err);
}

/*//////////////////////////////////////////////////////////////////////////
ASSERT APPROX EQ ABS
//////////////////////////////////////////////////////////////////////////*/

function assertApproxEqAbs(
uint256 a,
uint256 b,
uint256 maxDelta
) internal virtual {
uint256 delta = a > b ? a - b : b - a;

if (delta > maxDelta) {
emit log ("Error: a ~= b not satisfied [uint]");
emit log_named_uint (" Expected", b);
emit log_named_uint (" Actual", a);
emit log_named_uint (" Max Delta", maxDelta);
emit log_named_uint (" Delta", delta);
fail();
}
}

function assertApproxEqAbs(
uint256 a,
uint256 b,
uint256 maxDelta,
string memory err
) internal virtual {
uint256 delta = a > b ? a - b : b - a;

if (delta > maxDelta) {
emit log_named_string ("Error", err);
emit log_named_uint (" Expected", b);
emit log_named_uint (" Actual", a);
emit log_named_uint (" Max Delta", maxDelta);
emit log_named_uint (" Delta", delta);
fail();
}
}

function assertApproxEqAbs(
int256 a,
int256 b,
int256 maxDelta
) internal virtual {
int256 delta = a > b ? a - b : b - a;

if (delta > maxDelta) {
emit log ("Error: a ~= b not satisfied [int]");
emit log_named_int (" Expected", b);
emit log_named_int (" Actual", a);
emit log_named_int (" Max Delta", maxDelta);
emit log_named_int (" Delta", delta);
fail();
}
}

function assertApproxEqAbs(
int256 a,
int256 b,
int256 maxDelta,
string memory err
) internal virtual {
int256 delta = a > b ? a - b : b - a;

if (delta > maxDelta) {
emit log_named_string ("Error", err);
emit log_named_int (" Expected", b);
emit log_named_int (" Actual", a);
emit log_named_int (" Max Delta", maxDelta);
emit log_named_int (" Delta", delta);
fail();
}
}

/*//////////////////////////////////////////////////////////////////////////
ASSERT APPROX EQ REL
//////////////////////////////////////////////////////////////////////////*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but really just need one section header for all the assertions to differentiate them from the other helper methods. Maybe just called "Assertions"

Copy link
Contributor Author

@OliverNChalk OliverNChalk Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to collapse into ASSERTIONS section, only hesitation would be if we wanted to properly overload assertApproxEqRel and assertApproxEqAbs. Then we might want to group these guys in their own region, separate to the assertEq region.

For now if we don't plan to fully do all the overloads in this PR, happy to just collapse to one collection and move on

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only hesitation would be if we wanted to properly overload assertApproxEqRel and assertApproxEqAbs

By "properly overload" do you mean with types other than int256/uint256? If so, I think just supporting int256/uint256 would cover most cases and is sufficient.

Either way, both a single Assertions section or two sections (one for assertEqs, one for approx equals) is good with me. I don't feel strongly about this, but see the thumbs up from @ZeroEkkusu so will leave it up to you both 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ends up being 4 sections if you group overloaded functions together. This may be excessive and the argument for would be if we expect more overloads to come down the track - then this makes it blindingly obvious where to put them and whether they comply with existing style/format.

Will defer to @ZeroEkkusu to act as tie breaker and move this forward. For reference here is the commit where I group overloaded assertions together: 924456a

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea I had was to use the big headers to divide the main parts of the contract:
"STD-CHEATS", "STD-ASSERTIONS", "STD-ERRORS", "STD-STORAGE".

I like the order as is, assertEq --> assertApproxEqAbs... It's logical IMO. Just put assertFalse first. I don't think we need any dividers rn; we can always add them later, as that'd be a non-breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, summary of changes:

  • Collapse assertion code regions into one: 99a36d8
  • Re-order assertFalse: 3c15901
  • Add STD-CHEATS code region: 113b11c

Changes not done:

  • Did not add STD-ERRORS or STD-STORAGE as these are standalone libraries, not regions within contract Test


function assertApproxEqRel(
uint256 a,
uint256 b,
uint256 maxPercentDelta // An 18 decimal fixed point number, where 1e18 == 100%
) internal virtual {
if (b == 0) return assertEq(a, b); // If the expected is 0, actual must be too.

uint256 percentDelta = ((a > b ? a - b : b - a) * 1e18) / b;

if (percentDelta > maxPercentDelta) {
emit log ("Error: a ~= b not satisfied [uint]");
emit log_named_uint (" Expected", b);
emit log_named_uint (" Actual", a);
emit log_named_decimal_uint (" Max % Delta", maxPercentDelta, 18);
emit log_named_decimal_uint (" % Delta", percentDelta, 18);
fail();
}
}

function assertApproxEqRel(
uint256 a,
uint256 b,
uint256 maxPercentDelta, // An 18 decimal fixed point number, where 1e18 == 100%
string memory err
) internal virtual {
if (b == 0) return assertEq(a, b); // If the expected is 0, actual must be too.

uint256 percentDelta = ((a > b ? a - b : b - a) * 1e18) / b;

if (percentDelta > maxPercentDelta) {
emit log_named_string ("Error", err);
emit log_named_uint (" Expected", b);
emit log_named_uint (" Actual", a);
emit log_named_decimal_uint (" Max % Delta", maxPercentDelta, 18);
emit log_named_decimal_uint (" % Delta", percentDelta, 18);
fail();
}
}

function assertApproxEqRel(
int256 a,
int256 b,
int256 maxPercentDelta // An 18 decimal fixed point number, where 1e18 == 100%
) internal virtual {
if (b == 0) return assertEq(a, b); // If the expected is 0, actual must be too.

int256 percentDelta = ((a > b ? a - b : b - a) * 1e18) / b;

if (percentDelta > maxPercentDelta) {
emit log ("Error: a ~= b not satisfied [uint]");
emit log_named_int (" Expected", b);
emit log_named_int (" Actual", a);
emit log_named_decimal_int (" Max % Delta", maxPercentDelta, 18);
emit log_named_decimal_int (" % Delta", percentDelta, 18);
fail();
}
}

function assertApproxEqRel(
int256 a,
int256 b,
int256 maxPercentDelta, // An 18 decimal fixed point number, where 1e18 == 100%
string memory err
) internal virtual {
if (b == 0) return assertEq(a, b); // If the expected is 0, actual must be too.

int256 percentDelta = ((a > b ? a - b : b - a) * 1e18) / b;

if (percentDelta > maxPercentDelta) {
emit log_named_string ("Error", err);
emit log_named_int (" Expected", b);
emit log_named_int (" Actual", a);
emit log_named_decimal_int (" Max % Delta", maxPercentDelta, 18);
emit log_named_decimal_int (" % Delta", percentDelta, 18);
fail();
}
}
}


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a header here before stdError "STD-ERRORS" to separate them from the assertions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, can resolve if happy

Expand All @@ -159,7 +367,7 @@ library stdError {
struct StdStorage {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here "STD-STORAGE"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, can resolve if happy

mapping (address => mapping(bytes4 => mapping(bytes32 => uint256))) slots;
mapping (address => mapping(bytes4 => mapping(bytes32 => bool))) finds;

bytes32[] _keys;
bytes4 _sig;
uint256 _depth;
Expand All @@ -171,7 +379,7 @@ struct StdStorage {
library stdStorage {
event SlotFound(address who, bytes4 fsig, bytes32 keysHash, uint slot);
event WARNING_UninitedSlot(address who, uint slot);

Vm private constant vm_std_store = Vm(address(uint160(uint256(keccak256('hevm cheat code')))));

function sigs(
Expand All @@ -192,8 +400,8 @@ library stdStorage {
// if map struct, will be bytes32(uint256(keccak256(abi.encode(key1, keccak256(abi.encode(key0, uint(slot)))))) + structFieldDepth);
function find(
StdStorage storage self
)
internal
)
internal
returns (uint256)
{
address who = self._target;
Expand All @@ -212,7 +420,7 @@ library stdStorage {
(, bytes memory rdat) = who.staticcall(cald);
fdat = bytesToBytes32(rdat, 32*field_depth);
}

(bytes32[] memory reads, ) = vm_std_store.accesses(address(who));
if (reads.length == 1) {
bytes32 curr = vm_std_store.load(who, reads[0]);
Expand All @@ -239,7 +447,7 @@ library stdStorage {
(success, rdat) = who.staticcall(cald);
fdat = bytesToBytes32(rdat, 32*field_depth);
}

if (success && fdat == bytes32(hex"1337")) {
// we found which of the slots is the actual one
emit SlotFound(who, fsig, keccak256(abi.encodePacked(ins, field_depth)), uint256(reads[i]));
Expand All @@ -259,7 +467,7 @@ library stdStorage {
delete self._target;
delete self._sig;
delete self._keys;
delete self._depth;
delete self._depth;

return self.slots[who][fsig][keccak256(abi.encodePacked(ins, field_depth))];
}
Expand Down Expand Up @@ -343,7 +551,7 @@ library stdStorage {
delete self._target;
delete self._sig;
delete self._keys;
delete self._depth;
delete self._depth;
}

function bytesToBytes32(bytes memory b, uint offset) public pure returns (bytes32) {
Expand Down