Function Modifier or an Opcode to mark a test as skipped conditionally

[vminkov]

Component

Forge

Describe the feature you would like

It would be nice to have a Forge provided modifier that lets one mark a test as skipped [SKIP] depending on some custom condition. The specific use case we need it for is to mark which tests are run on different mainnet forks

The code that we currently use looks like this (below) but it marks the skipped tests as passed [PASS]

  uint256 BSC_MAINNET = 56;
  uint256 ETH_KOVAN = 42;
  uint256 ETH_RINKEBY = 4;
  uint256 EVMOS_TESTNET = 9000;

  /// custom modifier for running tests conditionally
  modifier shouldRun(bool run) {
    if (run) {
      _;
    }
  }

 // filter function for a specific chain
  function forChains(uint256 id0) public view returns (bool) {
    return block.chainid == id0;
  }

 // filter function for multiple chains
  function forChains(uint256 id0, uint256 id1) public view returns (bool) {
    return block.chainid == id0 || block.chainid == id1;
  }


  // TEST CASES


  function testSomething() public {
    // this test will be run on all chains
  }
 
  function testSomethingElse() public shouldRun(forChains(BSC, ETH_KOVAN)) {
    // this test will be skipped on EVMOS_TESTNET and ETH_RINKEBY
  }

I would imagine that we're provided by the forge env either a modifier like shouldRun(forChains(...)) or some assembly opcode to return from the test marking it as skipped [SKIP] and then we can craft the modifier in the same way

  modifier shouldRun(bool run) {
    if (run) {
      _;
    }
    else {
      assembly {
        some_op_code_to_mark_test_as_skipped()
      }
    }
  }

Additional context

[onbjerg]

Note on implementation: This could be a cheatcode (vm.skip(bool)) that simply reverts with some magic bytes. When a test reverts, we then check if the revert is the magic bytes, and if it is, we mark the test as skipped instead of failed

Others could then implement their custom modifiers, and forge-std could also have one, something like:

modifier shouldRun(bool pred) {
  vm.skip(!pred);
  _;
}

// Or
modifier shouldRunForChains(uint256 id0, uint256 id1) public view returns (bool) {
  vm.skip(block.chainid != id0 && block.chainid != id1);
  _;
}

The bulk of the work is not the cheatcode itself, but adjusting all of the test-related code to move away from success (true/false) to an enum

Marked it as low priority since it can already be solved using custom solutions, even if the outcome is not 100% desireable

[0x00000002]

    /**
     * @notice this modifier will skip the testFail*** ONLY
     */
    modifier skipFail(bool isSkipped) {
        if (!isSkipped) {
            require(0 == 1); 
            _;
        }
    }```

[onbjerg]

I wonder if this is needed anymore since we now have fork-related cheatcodes to choose what fork to use on a per-test level? Wdyt @vminkov? I can't really think of a use case beyond that.

[mds1]

I still think this feature is useful in some cases. For example with hardhat, I'd sometimes scaffold out a bunch of unimplemented tests and mark them as skipped to signal that they're not implemented. Having them pass/fail didn't feel correct since it might be a while until all tests are implemented.

Instead of a cheatcode, another idea is a special test name prefix, like function skipTest*() instead of the usual function test*(). (Either cheatcode or function name is fine with me, no preference here)

I don't feel too strongly about this feature and am ok marking this closed, pending @vminkov's thoughts

[vminkov]

For our use case the fork-related cheatcodes are good enough alternative. It is still useful to mark a test case as skipped when the reasons for its failure are not related to the code that is tested, but to the fork RPC URL being invalid, for example.