Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic MockApi for testing #423

Closed
TarikGul opened this issue Feb 7, 2021 · 4 comments
Closed

Dynamic MockApi for testing #423

TarikGul opened this issue Feb 7, 2021 · 4 comments
Labels
I8 - Enhancement Additional feature request I10 - Testing Sidecar needs tests added Q3 - Medium Requires decent knowledge of Sidecar/Substrate

Comments

@TarikGul
Copy link
Member

TarikGul commented Feb 7, 2021

Proposed Change or Idea

Create a more dynamic mockApi for our test suite. An example would be to turn the existing mockApi object, that has hardcoded values, into a function that would take a set of default key values and a set of option key values as it's parameters. I think this could be useful in having more readable testing, and would encourage a wider range of testing for edge cases.

For example something similar to this could work.

export const mockApi = (options, default=DEFAULT_POLKADOT_API_DATA) => {
    return {
        ...
        derive: {
            chain: {
		getHeader: options.derive.chain.getHeader ? options.derive.chain.getHeader : default.derive.chain.getHeader ,
		getBlock: options.derive.chain.getBlock ? options.derive.chain.getBlock : default.derive.chain.getBlock ,
	    },
	},
        ....
    }
}

This is how we currently get around an edge case by overriding values in the mockApi object

/**
 * CURRENT
 */               
it('Return an error with a null calcFee when perByte is undefined', async () => {
    mockApi.consts.transactionPayment.transactionByteFee = (undefined as unknown) as BalanceOf &
        AugmentedConst<'promise'>;

    const configuredBlocksService = new BlocksService(mockApi);

    // fetchBlock options
    const options = {
        eventDocs: true,
        extrinsicDocs: true,
        checkFinalized: false,
        queryFinalizedHead: false,
        omitFinalizedTag: false,
    };

    const response = sanitizeNumbers(
        await configuredBlocksService.fetchBlock(
            blockHash789629,
            options
        )
    );

    const responseObj: ResponseObj = JSON.parse(
        JSON.stringify(response)
    );

    // Revert mockApi back to its original setting that was changed above.
    mockApi.consts.transactionPayment.transactionByteFee = polkadotRegistry.createType(
        'Balance',
        1000000
    ) as BalanceOf & AugmentedConst<'promise'>;

    expect(responseObj.extrinsics[3].info).toEqual({
        error: 'Fee calculation not supported for 16#polkadot',
    });
});

This is how the proposed idea would look like

/**
 * PROPOSED
 */
it('Return an error with a null calcFee when perByte is undefined', async () => {
    const calcFeeOptions = {
        consts: {
            transactionPayment: {
                transactionByteFee = (undefined as unknown) as BalanceOf & AugmentedConst<'promise'>;
            }    
        }
    }

    const calcFeePerByteMockApi = mockApi(calcFeeOptions);
    
    const calcFeePerByteBlocksService = new BlocksService(calcFeePerByteMockApi);
    
    // fetchBlock options
    const options = {
        eventDocs: true,
        extrinsicDocs: true,
        checkFinalized: false,
        queryFinalizedHead: false,
        omitFinalizedTag: false,
    };

    const response = sanitizeNumbers(
        await calcFeePerByteBlocksService.fetchBlock(
            blockHash789629,
            options
        )
    );
    
    const responseObj: ResponseObj = JSON.parse(
        JSON.stringify(response)
    );

    expect(responseObj.extrinsics[3].info).toEqual({
        error: 'Fee calculation not supported for 16#polkadot',
    });
});

Alternatives

None I have thought of yet.

Additional Information

I think this could be a great way to organize the testing code more, and could be an enhancement for readability.

@TarikGul TarikGul added I8 - Enhancement Additional feature request Q3 - Medium Requires decent knowledge of Sidecar/Substrate labels Feb 8, 2021
@dvdplm
Copy link
Contributor

dvdplm commented Feb 12, 2021

So essentially this would allow us to skip resetting the value(s) under test; instead the mocks automatically use the defaults unless overridden. Did I get that right? Sounds like a good QoL improvement.

@TarikGul
Copy link
Member Author

Yup exactly, QoL improvement would definitely be the proper term. It would definitely be a large PR, but I think come the future it would encourage wider test coverage, and cleaner code.

@emostov
Copy link
Contributor

emostov commented Mar 9, 2021

@emostov emostov added the I10 - Testing Sidecar needs tests added label Mar 16, 2021
@TarikGul
Copy link
Member Author

#702 closes this issue, as we have moved the mockApi testing structure to integrate with a historicApi. The way I implemented it is similar to the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I8 - Enhancement Additional feature request I10 - Testing Sidecar needs tests added Q3 - Medium Requires decent knowledge of Sidecar/Substrate
Projects
None yet
Development

No branches or pull requests

3 participants