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

[Tests] Adding unit tests for GetDifficulty in blockchain.cpp. #11748

Merged
merged 1 commit into from Dec 23, 2017

Commits on Nov 22, 2017

  1. [Tests] Adding unit tests for GetDifficulty in blockchain.cpp.

    blockchain.cpp has low unit test coverage. This commit is intended
    to start improving its code coverage to reasonable levels. One or more
    follow up commits will complete the task that this commit is starting
    (though the usefulness of this commit is not dependent upon later
    commits).
    
    Note that these tests were not written based upon a specification of how
    GetDifficulty *should* work, but rather how it actually *does* work. As
    a result, if there are any bugs in the current GetDifficulty
    implementation, these unit tests serve to lock them in rather than
    expose them.
    
    -- Why has blockchain.cpp been modified if this is a unit testing change?
    
    Since the existing GetDifficulty function relies on a global variable,
    chainActive, it was not suitable for unit testing purposes. Both the
    existing GetDifficulty function and the unit tests now call through to
    a new, more modular version of GetDifficulty that can work on any chain,
    not just chainActive.
    
    -- Why does blockchain_tests.cpp directly include blockchain.cpp instead
    of blockchain.h?
    
    While the new GetDifficulty function's signature is arguably better than
    the old one's, it still isn't great, and doesn't seem to warrant inclusion
    as part of the blockchain.h API, especially since only test code is
    directly using it. If a better way of exposing the new GetDifficulty
    function to unit tests exists, please mention it and the commit will be
    updated accordingly.
    
    -- Why is the test fixture named blockchain_difficulty_tests rather than
    blockchain_tests?
    
    The Bitcoin Core policy for naming unit test files is to match the the
    file under test ("blockchain" becomes "blockchain_tests"). While this
    commit complies with that, blockchain.cpp is a massive file, such that
    having all of the unit tests in one file will tend towards disorder.
    Since there will be a lot more tests added to this file, the intention
    is to divide up different types of tests into different test fixtures
    within the same file.
    sean committed Nov 22, 2017
    1 Configuration menu
    Copy the full SHA
    3e1ee31 View commit details
    Browse the repository at this point in the history