Using bulloak to generate test contracts from tree

[smol-ninja]

Since there has been an ongoing discussion on using bulloak in slack, I decided to give it a go today. Bulloak uses branch descriptions to generate function names, so I decided to keep the description as minimum as possible to prevent it from creating long names.

As an example, I refactored the following tree:

withdrawableAmountOf.t.sol
├── given the id references a null stream
│  └── it should revert
└── given the id does not reference a null stream
   ├── given the stream balance is zero
   │  └── it should return zero
   └── given the stream balance is not zero
      ├── given the stream balance is less than the remaining amount
      │  └── it should return the balance
      └── given the stream balance is not less than the remaining amount
         ├── given the id references a paused stream
         │  └── it should return the remaining amount
         └── given the id does not reference a paused stream
            ├── given the sum of the streamed amount and the remaining amount is less than the balance
            │  └── it should return the balance
            └── given the sum of the streamed amount and the remaining amount is not less than the balance
               └── it should return the correct withdrawable amount 

into

WithdrawableAmountOf_Integration_Test
├── given null
│  └── it should revert
└── given not null
   ├── given canceled
   │  └── it should return the remaining amount
   └── given not canceled
      ├── given zero balance
      │  ├── given zero remainingAmount
      │  │  └── it should return zero
      │  └── given not zero remaining amount
      │     └── it should return the remaining amount
      └── given not zero balance
         ├── given balance greater than streamedAmount
         │  └── it should return the balance plus the remaining amount
         └── given balance not greater than streamedAmount
            └── it should return the correct withdrawable amount

which generated the following test

Click to expand test contract

contract WithdrawableAmountOf_Integration_Test {
    function test_RevertGiven_Null() external {
        // it should revert
    }

    modifier givenNotNull() {
        _;
    }

    function test_GivenCanceled() external givenNotNull {
        // it should return the remaining amount
    }

    modifier givenNotCanceled() {
        _;
    }

    modifier givenZeroBalance() {
        _;
    }

    function test_GivenZeroRemainingAmount() external givenNotNull givenNotCanceled givenZeroBalance {
        // it should return zero
    }

    function test_GivenNotZeroRemainingAmount() external givenNotNull givenNotCanceled givenZeroBalance {
        // it should return the remaining amount
    }

    modifier givenNotZeroBalance() {
        _;
    }

    function test_GivenBalanceGreaterThanStreamedAmount() external givenNotNull givenNotCanceled givenNotZeroBalance {
        // it should return the balance plus the remaining amount
    }

    function test_GivenBalanceNotGreaterThanStreamedAmount()
        external
        givenNotNull
        givenNotCanceled
        givenNotZeroBalance
    {
        // it should return the correct withdrawable amount
    }
}

I think It does decreases the developer experience but at the same time, it makes tests generation and check automated by using bulloak commands.

Wdyt @sablier-labs/solidity? Does shortening the tree, like above, look good?

[PaulRBerg]

Will review later. Just a tip - you can use <details/> tags to hide long code snippets like these. Use the slash command / in the editor.

[PaulRBerg]

Reviewed now.

I think It does decreases the developer experience but at the same time, it makes tests generation and check automated by using bulloak commands.

The automation benefits are greater than the (slight) decrease in developer experience.

[smol-ninja]

What do you think about the naming of the last function in the test contract? We currently use test_FunctionName to represent this.

In the above example, it would be function test_WithdrawableAmountOf. But with bulloak, its going to be the description of the last branch. i.e. function test_GivenBalanceNotGreaterThanStreamedAmount. I personally like it because it mimic the tree structure as it is. But would like to hear your view on this?

[PaulRBerg]

The new name provided by Bulloak is OK

[andreivladbrg]

I just read this discussion, and I am of the opinion that it doesn’t decrease the DX. But, the only problem I see is with the last test name, which doesn’t include the function name

[PaulRBerg]

I'm ok with the last function name not including the function name.

Feel free to open a feature request in Bulloak if you have a strong opinion.

[smol-ninja]

I suppose we all have agreed to it. Should I go ahead and refactor tests to follow bulloak?

[andreivladbrg]

yeah #132