Best practices for Branching Tree Technique (BTT)

[smol-ninja]

I have been working on refactoring the test code by following the best practices for Branching Tree Technique (BTT) as explained in the presentation- Testing Best Practices in Foundry by @PaulRBerg. The techniques aim at creating a tree structure that can explain test case scenarios even to a non-technical person. A simple example:

contract Integration_test {
  function test_RevertWhen_Null() external {...}
  
  modifier whenNotNull() {...}
  
  function test_GetSegments() external whenNotNull {...}
}

can be represented by the following tree structure.

├── when the id references a null stream
│  └── it should revert
└── when the id does not reference a null stream
   └── it should return the correct segments

There are two keywords to specify the states: When for function parameters and Given for contract states. As in the above example, Null is a functional parameter condition represented by the When keyword.

In a more complicated scenario such as

function test_RevertWhen_DelegateCalled {}

function test_RevertWhen_AmountTooHigh() external whenNotDelegateCalled {}

function test_RevertWhen_AssetNotFlashLoanable() external whenNotDelegateCalled whenAmountNotTooHigh {}

where DelegateCalled is technically not a contract state but a mode of execution, it becomes hard to categorize this into When and Given.

If we use Given for DelegateCalled, we can also use RevertGiven instead of RevertWhen for it but RevertWhen sounds friendlier as a function name while Given can be more appropriate as a modifier's name. (Given this, Revert when)

It, thus, can be refactored as the following:

function test_RevertWhen_DelegateCalled {}

function test_RevertWhen_AmountTooHigh() external givenNotDelegateCalled {}

function test_RevertWhen_AssetNotFlashLoanable() external givenNotDelegateCalled givenAmountNotTooHigh {}

This improves the readability of the tree structure in line with the naming convention used for function names, but brings in inconsistencies in the use of Given and When to specify states. Have a look at the below tree structure:

├── when delegate called
│  └── it should revert
└── given delegate not called
│  └── when amount too high
│     └── it should revert
│  └── given amount not too high
│     └── when asset not flash loanable
│          └── it should revert
│     └── when asset flash loanable
│          └── it should not revert

The goal of this discussion is to find the best approach for using English keywords to build a tree structure so that it can be automated with the naming conventions of the test functions.

[PaulRBerg]

Thank you for your recent contributions, @smol-ninja, as well as the detailed write-up. I see two intertwined problems here:

1. How should we name the "Revert" function when the term used in the tree is "Given"?
2. Should we use the term "Given" or "When" to refer to modes of execution such as call vs delegate call, msg.sender, etc.?

After pondering this topic for a while, my answers are:

1. We should name it test_RevertGiven - I know that RevertWhen sounds better, but RevertGiven is way, way, way clearer when the corresponding branch in three uses the word "given". I will poke Matt Solomon to change the regex used in ScopeLint and the Foundry best practices to Revert(When|Given)? - update: I opened an issue here: Expand the scope of the naming convention regex ScopeLift/scopelint#32
2. We should use "When" to refer to modes of execution because this would be in line with the Gherkin reference, which says:

"Given" steps are used to describe the initial context of the system ... It is typically something that happened in the past

The purpose of "Given" steps is to put the system in a known state before the user (or external system) starts interacting with the system (in the "When" steps).

Some additional points:

• I do not like the dichotomy between "when delegate called" and "given delegate not called"
• The latter should be "when not delegate called" ("not" comes before)
• In your example three, when should be used for "amount high", and given should be used for "asset flash loanable"

[PaulRBerg]

Any updates @smol-ninja? Will you be able to refactor your PR #642 in light of what we discussed here?

[smol-ninja]

Thank you for your ideas @PaulRBerg. I acknowledge your reply and find it appealing especially the use of "when not delegate called". While I still find test_RevertGiven a bit less friendly, it will grow on me over time. In spite of that, I agree to have the consistencies for the keywords When and Given across the tree structure and function names.

I will update & re-open the PR as soon as I can.

[PaulRBerg]

Thanks very much!

[alexfertel]

Added support for this in bulloak with alexfertel/bulloak#16