Merge #11748: [Tests] Adding unit tests for GetDifficulty in blockchain.cpp.
commit20166f8a448156f476f3e23825f59ebec36424a9
authorWladimir J. van der Laan <laanwj@gmail.com>
Sat, 23 Dec 2017 10:15:18 +0000 (23 11:15 +0100)
committerWladimir J. van der Laan <laanwj@gmail.com>
Sat, 23 Dec 2017 10:22:18 +0000 (23 11:22 +0100)
treede3c3d829ca66fc4be58d0bd1b8c93236a70ab8c
parent9bad8d64721768953cb9c18f30d48ec8c2757879
parent3e1ee310437f4c93113f6121425beffdc94702c2
Merge #11748: [Tests] Adding unit tests for GetDifficulty in blockchain.cpp.

3e1ee31 [Tests] Adding unit tests for GetDifficulty in blockchain.cpp. (sean)

Pull request description:

  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.

Tree-SHA512: a7dda9c2a9414d4819b4d2911f5637891dc19cecbecfc1463846161d2a78793151927a5ab911c69a5d3013f7668e75a1d78a65667cb9d83910cda439cbe84d62
src/Makefile.test.include
src/rpc/blockchain.cpp