test(refactor): use bulloak for concrete tests

smol-ninja commented 4 months ago

Linked issues

Closes https://github.com/sablier-labs/flow/issues/132

Additional changelog

getBlockTimestamp() in Utils
A few modifiers are marked as virtual. They are overridden by the modifiers with the same name in the test contracts to follow DRY. These identical modifiers in test contracts are used to change states across distinct test functions.
There were incorrect branches and tests which have been fixed.
Redundant modifiers have been removed.
Call to Helpers.sol functions are made in Integration to prevent importing it in each test contract.

Following tests have been refactored using bulloak:

[x] adjustRatePerSecond
[x] create
[x] deposit
[x] pause
[x] recentAmountOf
[x] refund
[x] refundableAmountOf
[x] restart
[x] restartAndDeposit
[x] streamDebtOf
[x] transferFrom
[x] withdrawAt
[x] withdrawMax
[x] withdrawableAmountOf

smol-ninja commented 4 months ago

@PaulRBerg I have also marked you as a reviewer (I know you are busy preparing for demo day so no rush), in case you want to have a look how the tests look like using Bulloak format.

@andreivladbrg since this PR could be a bit complicated to review, my advise would be to review trees first and discover any discrepancies there. The tests follow exactly the same structure as the trees.

PaulRBerg commented 4 months ago

Thanks guys, will review later

smol-ninja commented 4 months ago

I find it redundant to have the comment in a function called RevertWhen or RevertGiven, let's remove the comments:

Fair point. i agree with removing them.

but in more place you decided to remove the "is" - which i agree with and i suggest to remove it everywhere

I struggled with this a lot. The problem is that at some places, we need "is" and at other places we don't. For example, "when deposit amount is zero" sounds better than "when deposit amount zero". I will have a look and try to bring consistencies among them.

PS: I see that you have already taken care of it in your PR. Thanks!

SablierFlow_TransferAmountZero

Good finding. And thanks for creating feedback PR. I am going to review it now.

sablier-labs / flow