if-statementsoliditycode-coveragetest-coverage

How to test all branches of a range test?

While aiming at 100% branch coverage, I am experiencing some difficulties in testing both conditions of a range check:

// Find the matching tier
    for (uint256 i = 0; i < tiers.length; i++) {
      if (tiers[i].minVal() <= cumReceivedInvestments && cumReceivedInvestments < tiers[i].maxVal()) {
        return tiers[i];
      }
    }

As shown by LCOV: enter image description here

To test those conditions, I tried:

// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.23 <0.9.0;

import { PRBTest } from "@prb/test/src/PRBTest.sol";
import { StdCheats } from "forge-std/src/StdCheats.sol";
import { DecentralisedInvestmentHelper } from "../../src/Helper.sol";
import { TierInvestment } from "../../src/TierInvestment.sol";
import { Tier } from "../../src/Tier.sol";

contract HelperTest is PRBTest, StdCheats {
  TierInvestment internal validTierInvestment;

  uint256 private cumReceivedInvestments;

  Tier[] private _tiers;
  Tier[] private someTiers;
  DecentralisedInvestmentHelper private _helper;

  /// @dev A function invoked before each test case is run.
  function setUp() public virtual {
    cumReceivedInvestments = 5;

    // Specify the investment tiers in ether.
    uint256 firstTierCeiling = 4 ether;
    uint256 secondTierCeiling = 15 ether;
    uint256 thirdTierCeiling = 30 ether;

    // Start lowst tier at 2 wei, such that the tested cumulative investment
    //amount can go below that at 1 wei.
    Tier tier_0 = new Tier(2, firstTierCeiling, 10);
    _tiers.push(tier_0);
    Tier tier_1 = new Tier(firstTierCeiling, secondTierCeiling, 5);
    _tiers.push(tier_1);
    Tier tier_2 = new Tier(secondTierCeiling, thirdTierCeiling, 2);
    _tiers.push(tier_2);

    // Initialise contract helper.
    _helper = new DecentralisedInvestmentHelper();
  }

  function testExceedInvestmentCeiling() public {
    // vm.prank(address(validTierInvestment));
    vm.expectRevert(bytes("The investment ceiling is reached."));
    _helper.computeCurrentInvestmentTier(30 ether + 1 wei, _tiers);
  }

  function testNegativeInvestment() public {
    // vm.prank(address(validTierInvestment));
    vm.expectRevert(
      bytes(
        "Unexpected state: No matching tier found, the lowest investment tier starting point was larger than the cumulative received investments. All (Tier) arrays should start at 0."
      )
    );
    _helper.computeCurrentInvestmentTier(1 wei, _tiers);
  }

  function testCanInvestInNextTier() public {
    // True True for tier 0.
    assertEq(_helper.computeCurrentInvestmentTier(2 wei, _tiers).multiple(), 10);

    // False False for tier 0.
    assertEq(_helper.computeCurrentInvestmentTier(10 ether + 1 wei, _tiers).multiple(), 5);

    // Hits investment ceiling
    // assertEq(_helper.computeCurrentInvestmentTier(30 ether+1 wei, _tiers).multiple(), 2);

    // True True for tier 0, True True for tier 1 but tier 1 is not reached.,
    assertEq(_helper.computeCurrentInvestmentTier(2 wei, _tiers).multiple(), 10);

    // Hits investment ceiling before this can reach Tier 0.
    // False True for tier 0, True True for tier 1
    // assertEq(_helper.computeCurrentInvestmentTier(1 wei, _tiers).multiple(), 10);
  }

  function testGetRemainingAmountInCurrentTierBelow() public {
    vm.expectRevert(bytes("Error: Tier's minimum value exceeds received investments."));
    _helper.getRemainingAmountInCurrentTier(1 wei, _tiers[0]);
  }

  function testGetRemainingAmountInCurrentTierAbove() public {
    vm.expectRevert(bytes("Error: Tier's maximum value is not larger than received investments."));
    _helper.getRemainingAmountInCurrentTier(4 ether + 1 wei, _tiers[0]);
  }

  function testComputeRemainingInvestorPayoutNegativeFraction() public {
    vm.expectRevert(bytes("investorFracNumerator is smaller than investorFracDenominator."));
    _helper.computeRemainingInvestorPayout(0, 1, 0, 0);
  }
}

Which I believe tests the case:

False && True

tiers[i].minVal() <= cumReceivedInvestments = false
cumReceivedInvestments < tiers[i].maxVal() = true

for: tier 0, and cumReceivedInvestments = 10 ether+1 wei.

True && True

tiers[i].minVal() <= cumReceivedInvestments = true
cumReceivedInvestments < tiers[i].maxVal() = true

for: tier 0, and cumReceivedInvestments = 2 wei.

False && False

tiers[i].minVal() <= cumReceivedInvestments = false
cumReceivedInvestments < tiers[i].maxVal() = false

for: tier 0, and cumReceivedInvestments = 10+1 wei.

True && False

However, I don't think I can realise:

tiers[i].minVal() <= cumReceivedInvestments = true
cumReceivedInvestments < tiers[i].maxVal() = false

because the Tier object requires that the maxVal is larger than the minVal, so if the first line is true, inherently the cumReceivedInvestments < tiers[i].maxVal() is also true.

Additional Attempt

I thought if I separate that logic into a separate function, and throw all the test cases in that function, then I'll be able to cover all cases. That was not the case, and I think I incorrectly assumed it was the True && False case that I can not realise. Perhaps LCOV says it is not covered due to the for loop for some reason. For completeness, here is the separated function:

function isInRange(uint256 minVal, uint256 maxVal, uint256 someVal) public view override returns (bool inRange) {
    if (minVal <= someVal && someVal < maxVal) {
      inRange = true;
    } else {
      inRange = false;
    }
    return inRange;
  }

  function computeCurrentInvestmentTier(
    uint256 cumReceivedInvestments,
    Tier[] memory tiers
  ) public view override returns (Tier currentTier) {
    // Check for exceeding investment ceiling.
    if (hasReachedInvestmentCeiling(cumReceivedInvestments, tiers)) {
      revert ReachedInvestmentCeiling(cumReceivedInvestments, "Investment ceiling is reached.");
    }

    // Find the matching tier
    uint256 nrOfTiers = tiers.length;
    for (uint256 i = 0; i < nrOfTiers; ++i) {
      if (isInRange(tiers[i].getMinVal(), tiers[i].getMaxVal(), cumReceivedInvestments)) {
        currentTier = tiers[i];
        return currentTier;
      }
    }
    // Should not reach here with valid tiers
    revert(
      string(
        abi.encodePacked(
          "Unexpected state: No matching tier found, the lowest ",
          "investment tier starting point was larger than the ",
          "cumulative received investments. All (Tier) arrays should start at 0."
        )
      )
    );
  }

and test:

function testCanInvestInNextTier() public override {
    // True True for tier 0.
    assertEq(_helper.computeCurrentInvestmentTier(2 wei, _tiers).getMultiple(), 10);
    assertTrue(_helper.isInRange(1, 3, 2));

    // False False for tier 0.
    assertEq(_helper.computeCurrentInvestmentTier(10 ether + 1 wei, _tiers).getMultiple(), 5);
    assertFalse(_helper.isInRange(1, 2, 4));

    // Hits investment ceiling
    // assertEq(_helper.computeCurrentInvestmentTier(30 ether+1 wei, _tiers).getMultiple(), 2);

    // True True for tier 0, True True for tier 1 but tier 1 is not reached.,
    assertEq(_helper.computeCurrentInvestmentTier(2 wei, _tiers).getMultiple(), 10);
    assertFalse(_helper.isInRange(1, 2, 0));
    assertFalse(_helper.isInRange(3, 2, 1));

    // Hits investment ceiling before this can reach Tier 0.
    // False True for tier 0, True True for tier 1
    // assertEq(_helper.computeCurrentInvestmentTier(1 wei, _tiers).getMultiple(), 10);
  }

And the accompanying LCOV report: enter image description here

Question

Assuming I want 100% test coverage, how could I resolve this issue? For example should I change the if condition, should I move or change the require statement, or is my analysis incorrect, and is it possible to test all 4 conditions of the && condition, or perhaps something else?

Whole function

For completeness, here is the complete function that is being tested:

function hasReachedInvestmentCeiling(uint256 cumReceivedInvestments, Tier[] memory tiers) public view returns (bool) {
    return cumReceivedInvestments >= getInvestmentCeiling(tiers);
  }

  function computeCurrentInvestmentTier(
    uint256 cumReceivedInvestments,
    Tier[] memory tiers
  ) public view returns (Tier) {
    // Check for exceeding investment ceiling.

    require(!hasReachedInvestmentCeiling(cumReceivedInvestments, tiers), "The investment ceiling is reached.");

    // Find the matching tier
    for (uint256 i = 0; i < tiers.length; i++) {
      if (tiers[i].minVal() <= cumReceivedInvestments && cumReceivedInvestments < tiers[i].maxVal()) {
        return tiers[i];
      }
    }
    // Should not reach here with valid tiers
    revert(
      "Unexpected state: No matching tier found, the lowest investment tier starting point was larger than the cumulative received investments. All (Tier) arrays should start at 0."
    );
  }
Solution

  • You can rewrite this as a while loop and then have 2 separate checks. I assume that tiers array is sorted and sound - i.e. tiers are not overlapping and don't have holes between.

    For loop termination you check the min and for increment you check the max.

    I'm not familiar with this Sol language, but it looks C-like so I'd suggest something like this:

    uint256 i = 0;
while(tiers[i].minVal() <= cumReceivedInvestments) { 
  if (cumReceivedInvestments < tiers[i].maxVal()) {
    i++;
  }
}

return tiers[i];

    This way each branch will be tested and code still achieves same goal.