Test doesn't revert as expected in lesson 9

[miladad8]

I've been playing around with my test and I don't get it why it doesn't revert. In HelperConfig file lesson 9 when we are trying to set our Anvil chain configuration, first we check:

        if (activeNetworkConfig.vrfCoordinator != address(0)) {
            return activeNetworkConfig;
        }

I am trying to write a test for this line of code to check that we couldn't deploy a new vrfCoordinatorMock if we already deployed one and if we try to deploy one we should expect a revert. below is my test:

    function testCantDeployVrfcoordinatorMockIfExists() public {
        (
            uint256 _entranceFee,
            uint256 _interval,
            address _vrfCoordinator,
            bytes32 _gasLane,
            uint64 _subscriptionId,
            uint32 _callbackGasLimit,
            address _link
        ) = helperConfig.activeNetworkConfig();

        uint96 baseFee = 0.25 ether;
        uint96 gasPriceLink = 1e9;

        if (_vrfCoordinator != address(0)) {
            vm.expectRevert();
            vm.prank(USER);
            new VRFCoordinatorV2Mock(baseFee, gasPriceLink);
        }
    }

But test fails. However when I set _vrfCoordinator == address(0) , the test passed successfully.
I was thinking it should be reversed. what am I missing here?

Any help would be appreciated

[EngrPips]

Hello @miladad8, The test is failing because of how you set it up. Let's understand what your test is doing. I will comment on your test to explain what your test does.

function testCantDeployVrfcoordinatorMockIfExists() public {
//Here, you are getting out all the values in the `activeNetworkConfig.` This means you have initialized an `HelperConfig` in your `setup` function, most likely
        (
            uint256 _entranceFee,
            uint256 _interval,
            address _vrfCoordinator,
            bytes32 _gasLane,
            uint64 _subscriptionId,
            uint32 _callbackGasLimit,
            address _link
        ) = helperConfig.activeNetworkConfig();
// Here, you declare a variable named `baseFee,` and you set it to `0.25 ether.`
        uint96 baseFee = 0.25 ether;
// Here, you declared a variable named `gasPriceLink` and set it to `1e9.`
        uint96 gasPriceLink = 1e9;
// Here, you check to see if the `_vrfCoordinator` variable you get from the `helperConfig.activeNetworkConfig()` is not `address(0)`, If so you enter the code of block
        if (_vrfCoordinator != address(0)) {
// Here, you tell your test to expect a revert, so now the test is patiently waiting for some transaction to revert so that your test will pass
            vm.expectRevert();
// Here you prank `USER` to send the transaction that comes next
            vm.prank(USER);
// Here, you created a new `VRFCoordinatorV2Mock` and passed in all the necessary arguments to initialize a contract of type `VRFCoordinatorV2Mock`. The contract was created, and the transaction didn't revert 
            new VRFCoordinatorV2Mock(baseFee, gasPriceLink);
// Your whole process finishes running, and nothing reverts while you have told your test to expect a revert. Your test concluded that the test failed since it didn't revert as you expected.
        }

Testing the code below can be tricky.

 if (activeNetworkConfig.vrfCoordinator != address(0)) {
            return activeNetworkConfig;
        }

[EngrPips]

This line of code new VRFCoordinatorV2Mock(baseFee, gasPriceLink); in your test has no connection whatsoever with your HelperConfig.

[miladad8]

I have imported VRFCoordinatorV2Mock above my test file. However I didn't show it above. I just show the test function here. If that you means.

[EngrPips]

I know you have imported VRFCoordinatorV2Mock before you were able to do new VRFCoordinatorV2Mock(baseFee, gasPriceLink); but think about it, how does doing new VRFCoordinatorV2Mock(baseFee, gasPriceLink); affects the HelperConfig contract?

[miladad8]

Why does it even have to do anything about HelperConfig ? I thought we only have to care about our test if they are forked or unit or chain we are testing. Patrick has written a lot of integration test during the course.

[EngrPips]

What are you writing the test for? I mean, this test below what is it testing?

function testCantDeployVrfcoordinatorMockIfExists() public {
//Here, you are getting out all the values in the `activeNetworkConfig.` This means you have initialized an `HelperConfig` in your `setup` function, most likely
        (
            uint256 _entranceFee,
            uint256 _interval,
            address _vrfCoordinator,
            bytes32 _gasLane,
            uint64 _subscriptionId,
            uint32 _callbackGasLimit,
            address _link
        ) = helperConfig.activeNetworkConfig();
// Here, you declare a variable named `baseFee,` and you set it to `0.25 ether.`
        uint96 baseFee = 0.25 ether;
// Here, you declared a variable named `gasPriceLink` and set it to `1e9.`
        uint96 gasPriceLink = 1e9;
// Here, you check to see if the `_vrfCoordinator` variable you get from the `helperConfig.activeNetworkConfig()` is not `address(0)`, If so you enter the code of block
        if (_vrfCoordinator != address(0)) {
// Here, you tell your test to expect a revert, so now the test is patiently waiting for some transaction to revert so that your test will pass
            vm.expectRevert();
// Here you prank `USER` to send the transaction that comes next
            vm.prank(USER);
// Here, you created a new `VRFCoordinatorV2Mock` and passed in all the necessary arguments to initialize a contract of type `VRFCoordinatorV2Mock`. The contract was created, and the transaction didn't revert 
            new VRFCoordinatorV2Mock(baseFee, gasPriceLink);
// Your whole process finishes running, and nothing reverts while you have told your test to expect a revert. Your test concluded that the test failed since it didn't revert as you expected.
        }

[EngrPips]

If anything is Confusing, please ask questions.

[EngrPips]

 if (activeNetworkConfig.vrfCoordinator != address(0)) {
            return activeNetworkConfig;
        }

The line of code checks if there is already an existing vrfCoordinator for the AnvilChainConfig. If so, that means there is a NetworkConfig for anvil already, and we just return that NetworkConfig, but if the vrfCoordinator on Anvil is address(0) that means there is no NetworkConfig for anvil yet then we go ahead and set up a NetworkConfig for Anvil chain.

[miladad8]

That's right, thanks! So how can we test that if statement in HelperConfig? Apparently there is no easy way to test it.

[EngrPips]

Testing that line is tricky considering the current architecture of the HelperConfig codebase because it won't revert if there isn't a NetworkConfig for Anvil yet. Instead, it will create a new one. What makes it even more challenging to test the code is that the NetworkConfig gets initialized right in the constructor, so the moment you initiate a HelperConfig contract, it does the checks. It sets up a NetworkConfig if needed. You can change the architecture of the codebase to make it suitable for this testing you intend to do if you are determined to do it, but it would require changing a lot of stuff in some other contracts as well and can be confusing if you aren't deeply familiar with writing smart contracts

[miladad8]

thank you for your time!

[EngrPips]

You are welcome. Friend.