How should be addressed common testing scenarios

[clauBv23]

When testing complex code a common scenario scenario involves verifying specific behaviors under different setups. For example in Sablier V2 Core, the test tree looks like this:

             ...
            └── given all streams are warm
            ├── when the caller is unauthorized for all streams
            │  ├── when the caller is a malicious third party
            │  │  └── it should revert
            │  └── when the caller is the recipient
            │     └── it should revert
            ├── when the caller is unauthorized for some streams
            │  ├── when the caller is a malicious third party
            │  │  └── it should revert
            │  └── when the caller is the recipient
            │     └── it should revert
            ...

This tree structure checks that when the caller is a malicious third party and when the caller is the recipient should revert in both cases: when the caller is unauthorized for all streams and when the caller is unauthorized for some streams.

Using bulloak, this specification fails because in Solidity two functions or modifiers cannot have the same signature.

However, in the Sablier V2 Core repo this issue is managed by also appending the scenario description to the function name instead of just creating a modifier, as shown below:

function test_RevertWhen_CallerUnauthorizedAllStreams_MaliciousThirdParty()
        external
        whenNotDelegateCalled
        whenArrayCountNotZero
        givenNoNull
        givenAllStreamsWarm
        whenCallerUnauthorized
    

Is there a way to handle this in bulloak? or any workaround for this issue?

Perhaps a special word, character, or ASCII sequence in the tree, could be used to append the description to the function name?

[alexfertel]

Yeah, this has been discussed a few times, and I don't have a good answer, tbh.

In this specific example, I would just "merge" both parent branches to something like when the caller is unauthorized to more than one stream. However, this only works when the whole sub-tree is common between the two branches.

There are a couple of solutions we could explore:

• Including the parent condition as a suffix.
  • Note that in the Sablier example, they don't generate the modifier, because those are two nested, consecutive conditions. If the parent when had two children, the modifier would be generated.
• Adding a keyword to the end of the condition.
  • What are the advantages of this vs e.g. rewording the condition or adding a 1 at the end?

I think #1 makes sense, and I'd be down to implement it, though I want to note that there is some value in pointing out that there are branches named the same. Also, some people hate long function names :(

wdyt?

[clauBv23]

Note that in the Sablier example, they don't generate the modifier, because those are two nested, consecutive conditions. If the parent when had two children, the modifier would be generated.

What do you mean?

If I understood correctly whenCallerUnauthorized is the modifier for when the caller is unauthorized for all streams. The name is not exactly the same but assumed it was due to it not being generated but manually created. The generated one should be something like whenTheCallerIsUnauthorizedForSomeStreams.

The parent when the caller is unauthorized for all streams condition has to children:
1 - when the caller is a malicious third party
2 - when the caller is the recipient

I think #1 makes sense, and I'd be down to implement it, though I want to note that there is some value in pointing out that there are branches named the same. Also, some people hate long function names :(

I totally agree that large function names should not become a common practice. However, there are scenarios where it could be really helpful, and maybe this is one of those cases. That's why I suggested providing the option to add the condition as a suffix. This way, it won't be applied in all cases, only when necessary.

I was thinking of something like this :

             ...
            └── given all streams are warm
              ├── when the caller is unauthorized for all streams __append__
              │  ├── when the caller is a malicious third party
              │  │  └── it should revert
              │  └── when the caller is the recipient
              │     └── it should revert
              ├── when the caller is unauthorized for some streams __append__
              │  ├── when the caller is a malicious third party
              │  │  └── it should revert
              │  └── when the caller is the recipient
              │     └── it should revert
            ...

I have no idea if this is possible or not, but the idea will be to add some sequence in the condition that will imply adding the condition description to the function's name.
This way, the condition will only be appended when necessary, avoiding long function names in general but allowing for clarity in specific complex scenarios.

[alexfertel]

What do you mean?

Ah, you are right, I got confused. Ignore that comment.

However, there are scenarios where it could be really helpful, and maybe this is one of those cases.

Yup yup, I agree.

I have no idea if this is possible or not, but the idea will be to add some sequence in the condition that will imply adding the condition description to the function's name.

Yeah, this is definitely possible. It needs a bit of sketching, but it should look pretty similar to your example.

I think maybe even something more general could be implemented, like specifying the title of the condition, which would solve #37 as well.

I'll give this a shot, though it may take some time. A workaround for now is rewording the condition.

[alexfertel]

Opened #78 to track this work

[clauBv23]

Awesome, can't wait to start playing with this 🙌

Thanks !!!

[clauBv23]

Thinking again about this.

Currently, if a parent has more than one child, a unique modifier is created for each condition. However, having the same modifier description more than once is not allowed, as it implies multiple modifiers with the same name are needed.

but, in most cases, the same description should correspond to the same modifier. If the modifier implementation differs, then the description should differ as well.

So, instead of disallowing duplicate descriptions, why not allow them and reuse the same modifier? In the end, the same is done when grouping under the same parent.

Consider the following example with conditions A, B, and C:

├── when A
│   ├── when B 1
│   │   ├── when C 1
│   │   │   └── it should do something.
│   │   └── when not C 1
│   │       └── it should do something.
│   └── when not B 1
│       ├── when C 2
│       │   └── it should do something.
│       └── when not C 2
│           └── it should do something.
└── when not A
    ├── when B 2
    │   ├── when C 3
    │   │   └── it should do something.
    │   └── when not C 3
    │       └── it should do something.
    └── when not B 2
        ├── when C 4
        │   └── it should do something.
        └── when not C 4
            └── it should do something.

In this structure:

• when C and when not C need renaming since multiple functions with the same name are not allowed.
• when B and when not B also require renaming, even though the same modifier could be reused, just as is done with when A and when not A.

By reusing modifiers with the same description, we can simplify the structure and avoid unnecessary renaming.