Unit tests for uint256.h #3422

tholenst commented at 7:25 PM on December 15, 2013: contributor

Unit tests for uint256.h. The file uint160_tests.cpp is no longer needed. The ad-hoc tests which were in uint256.h are also no longer needed. The new tests achieve 100% coverage.

in src/test/uint256_tests.cpp:None in b03ca188f3 outdated

  13 |  
  14 | -#include <boost/test/unit_test.hpp>
  15 | +const unsigned char R1Array[] = 
  16 | +    "\x9c\x52\x4a\xdb\xcf\x56\x11\x12\x2b\x29\x12\x5e\x5d\x35\xd2\xd2"
  17 | +    "\x22\x81\xaa\xb5\x33\xf0\x08\x32\xd5\x56\xb1\xf9\xea\xe5\x1d\x7d";
  18 | +const uint256 R1L = uint256(std::vector<unsigned char>(&R1Array[0],&R1Array[0]+32));

sipa commented at 8:52 PM on December 15, 2013:

&R1Array[32] instead?

tholenst commented at 8:58 PM on December 15, 2013:

I think that wouldn't be proper. If I understand C correctly, the string constant above is null terminated, and so has 33 elements.

sipa commented at 8:59 PM on December 15, 2013:

a[b] is semantically equal to *(a+b). Not sure what the string constant definition has to do with it.

tholenst commented at 9:00 PM on December 15, 2013:

Oh sorry, I read the wrong line :) Yes, you're right.

in src/test/uint256_tests.cpp:None in b03ca188f3 outdated

  59 | +void LowestUint64OfArray(const unsigned char A[],uint64_t& R1a)
  60 | +{
  61 | +    R1a = 0;
  62 | +    for (unsigned int i = 0; i < 8; ++i) 
  63 | +    {
  64 | +	R1a += (((uint64_t)A[i]) << 8*i);

sipa commented at 8:53 PM on December 15, 2013:

Indentation.

in src/test/uint256_tests.cpp:None in b03ca188f3 outdated

 455 | +    TmpL = R1L;
 456 | +    BOOST_CHECK(--TmpL == R1L-1);
 457 | +
 458 | +    // 160-bit; copy-pasted
 459 | +    uint160 TmpS = 0;
 460 | +    BOOST_CHECK(R1S+R2S == uint160("58F1B1369288D204211CA751527CFC175767850C"));

sipa commented at 8:56 PM on December 15, 2013:

Maybe make this a constant defined at the beginning of the file too, so it's clear that changing one must mean changing this too?

tholenst commented at 8:56 PM on December 15, 2013:

OK, there are a few more of those.

sipa commented at 8:57 PM on December 15, 2013: member

Very nice, just a few nits. There are several indentation errors though (tabs/spaces? please only use 4 spaces for indent).

tholenst closed this on Dec 15, 2013

tholenst commented at 9:14 PM on December 15, 2013: contributor

Closed while minor points are being fixed.

sipa commented at 9:20 PM on December 15, 2013: member

No need to close for minor fixes. You can just re-push to the same branch to update the pull request.

tholenst commented at 9:37 PM on December 15, 2013: contributor

Yeah, problem is I won't be able to finish this until Wednesday, no need to clutter things until then.

tholenst reopened this on Dec 18, 2013

in src/uint256.h:None in d1490c69b3 outdated

 637 | -
 638 | -
 639 | -
 640 | -
 641 | -
 642 | -#ifdef TEST_UINT256

laanwj commented at 7:59 AM on December 20, 2013:

Yes, let's remove this.

laanwj commented at 8:03 AM on December 20, 2013: member

Changes look good.

But before this can be merged, please reorganize the commits a bit.

You don't need to squash everything into one commit. For example d1490c6 and d1490c6 make sense as seperate ones, but do roll the dummy commit and indentation fixes into your original code changes.

tholenst commented at 4:00 PM on December 21, 2013: contributor

Thank you, Wladimir.

I'm new to github and git, so I don't know yet how to do useful commit structures. I improved it. Let me know if you still see room for improvement.

Changed Get64(.) to GetLow64()

The function Get64(.) has a bug in case the width is not divisible by 64.
Since it is only ever used as Get64(0) this simply changes it to this
special case. Additionally, an assert is added, and a cast to prevent
a compiler error.

e85e19be06

Unittests for uint256.h

Unit tests for uint256.h. The file uint160_tests.cpp is no longer
needed. The ad-hoc tests which were in uint256.h are also no longer
needed. The new tests achieve 100% coverage.

daa6b20e29

BitcoinPullTester commented at 10:40 AM on December 25, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/daa6b20e29f3926a16d4da6962ff00b74623fc6b for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj referenced this in commit ab086e0bd3 on Jan 6, 2014

laanwj merged this on Jan 6, 2014

laanwj closed this on Jan 6, 2014

DrahtBot locked this on Sep 8, 2021