Convert Checksum test to DEF_TEST() macro.

tests/ChecksumTest.cpp

-
 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
-#include "Test.h"
 
 #include "SkChecksum.h"
+#include "SkRandom.h"
+#include "Test.h"
+#include "TestClassDef.h"
 
-// Word size that is large enough to hold results of any checksum type.
-typedef uint64_t checksum_result;
+// Murmur3 has an optional third seed argument, so we wrap it to fit a uniform type.
+static uint32_t murmur_noseed(const uint32_t* d, size_t l) { return SkChecksum::Murmur3(d, l); }
 
-namespace skiatest {
-    class ChecksumTestClass : public Test {
-    public:
-        static Test* Factory(void*) {return SkNEW(ChecksumTestClass); }
-    protected:
-        virtual void onGetName(SkString* name) { name->set("Checksum"); }
-        virtual void onRun(Reporter* reporter) {
-            this->fReporter = reporter;
-            RunTest();
-        }
-    private:
-        enum Algorithm {
-            kSkChecksum,
-            kMurmur3,
-        };
+#define ASSERT(x) REPORTER_ASSERT(r, x)
 
-        // Call Compute(data, size) on the appropriate checksum algorithm,
-        // depending on this->fWhichAlgorithm.
-        checksum_result ComputeChecksum(const char *data, size_t size) {
-            switch(fWhichAlgorithm) {
-            case kSkChecksum:
-                REPORTER_ASSERT_MESSAGE(fReporter,
-                                        reinterpret_cast<uintptr_t>(data) % 4 == 0,
-                                        "test data pointer is not 32-bit aligned");
-                REPORTER_ASSERT_MESSAGE(fReporter, SkIsAlign4(size),
-                                        "test data size is not 32-bit aligned");
-                return SkChecksum::Compute(reinterpret_cast<const uint32_t *>(data), size);
-            case kMurmur3:
-                REPORTER_ASSERT_MESSAGE(fReporter,
-                                        reinterpret_cast<uintptr_t>(data) % 4 == 0,
-                                        "test data pointer is not 32-bit aligned");
-                REPORTER_ASSERT_MESSAGE(fReporter, SkIsAlign4(size),
-                                        "test data size is not 32-bit aligned");
-                return SkChecksum::Murmur3(reinterpret_cast<const uint32_t *>(data), size);
-            default:
-                SkString message("fWhichAlgorithm has unknown value ");
-                message.appendf("%d", fWhichAlgorithm);
-                fReporter->reportFailed(message);
-            }
-            // we never get here
-            return 0;
-        }
-
-        // Confirm that the checksum algorithm (specified by fWhichAlgorithm)
-        // generates the same results if called twice over the same data.
-        void TestChecksumSelfConsistency(size_t buf_size) {
-            SkAutoMalloc storage(buf_size);
-            char* ptr = reinterpret_cast<char *>(storage.get());
-
-            REPORTER_ASSERT(fReporter,
-                            GetTestDataChecksum(8, 0) ==
-                            GetTestDataChecksum(8, 0));
-            REPORTER_ASSERT(fReporter,
-                            GetTestDataChecksum(8, 0) !=
-                            GetTestDataChecksum(8, 1));
-
-            sk_bzero(ptr, buf_size);
-            checksum_result prev = 0;
-
-            // assert that as we change values (from 0 to non-zero) in
-            // our buffer, we get a different value
-            for (size_t i = 0; i < buf_size; ++i) {
-                ptr[i] = (i & 0x7f) + 1; // need some non-zero value here
-
-                // Try checksums of different-sized chunks, but always
-                // 32-bit aligned and big enough to contain all the
-                // nonzero bytes.  (Remaining bytes will still be zero
-                // from the initial sk_bzero() call.)
-                size_t checksum_size = (((i/4)+1)*4);
-                REPORTER_ASSERT(fReporter, checksum_size <= buf_size);
-
-                checksum_result curr = ComputeChecksum(ptr, checksum_size);
-                REPORTER_ASSERT(fReporter, prev != curr);
-                checksum_result again = ComputeChecksum(ptr, checksum_size);
-                REPORTER_ASSERT(fReporter, again == curr);
-                prev = curr;
-            }
-        }
-
-        // Return the checksum of a buffer of bytes 'len' long.
-        // The pattern of values within the buffer will be consistent
-        // for every call, based on 'seed'.
-        checksum_result GetTestDataChecksum(size_t len, char seed=0) {
-            SkAutoMalloc storage(len);
-            char* start = reinterpret_cast<char *>(storage.get());
-            char* ptr = start;
-            for (size_t i = 0; i < len; ++i) {
-                *ptr++ = ((seed+i) & 0x7f);
-            }
-            checksum_result result = ComputeChecksum(start, len);
-            return result;
-        }
-
-        void RunTest() {
-            const Algorithm algorithms[] = { kSkChecksum, kMurmur3 };
-            for (size_t i = 0; i < SK_ARRAY_COUNT(algorithms); i++) {
-                fWhichAlgorithm = algorithms[i];
-
-                // Test self-consistency of checksum algorithms.
-                TestChecksumSelfConsistency(128);
-
-                // Test checksum results that should be consistent across
-                // versions and platforms.
-                REPORTER_ASSERT(fReporter, ComputeChecksum(NULL, 0) == 0);
-
-                const bool colision1 = GetTestDataChecksum(128) == GetTestDataChecksum(256);
-                const bool colision2 = GetTestDataChecksum(132) == GetTestDataChecksum(260);
-                if (fWhichAlgorithm == kSkChecksum) {
-                    // TODO: note the weakness exposed by these collisions...
-                    // We need to improve the SkChecksum algorithm.
-                    // We would prefer that these asserts FAIL!
-                    // Filed as https://code.google.com/p/skia/issues/detail?id=981
-                    // ('SkChecksum algorithm allows for way too many collisions')
-                    REPORTER_ASSERT(fReporter, colision1);
-                    REPORTER_ASSERT(fReporter, colision2);
-                } else {
-                    REPORTER_ASSERT(fReporter, !colision1);
-                    REPORTER_ASSERT(fReporter, !colision2);
-                }
-            }
-        }
-
-        Reporter* fReporter;
-        Algorithm fWhichAlgorithm;
+DEF_TEST(Checksum, r) {
+    // Algorithms to test.  They're currently all uint32_t(const uint32_t*, size_t).
+    uint32_t(*kAlgorithms[])(const uint32_t*, size_t) = {

[tfarina, 2014/01/08 18:32:40]

array of pointers to functions, the beauty of C :)

[mtklein, 2014/01/08 18:41:55]

Yeah, this always takes me forever to remember how to even type it in.

[bungeman-skia, 2014/01/09 16:08:33]

I know that doing this inline is awesome and all, but maybe make this a typedef?

typedef uint32_t (*algorithmProc)(const uint32_t*, size_t);
algorithmProc kAlgorithms[] = { ...

[mtklein, 2014/01/09 16:21:46]

Done.

+        &SkChecksum::Compute,
+        &murmur_noseed,
     };
 
-    static TestRegistry gReg(ChecksumTestClass::Factory);
+    // Put 128 random bytes into two identical buffers.  Any multiple of 4 will do.
+    const size_t kBytes = 128;
+    SkRandom rand;
+    uint32_t data[kBytes/4], tweaked[kBytes/4];
+    for (size_t i = 0; i < SK_ARRAY_COUNT(tweaked); i++) data[i] = tweaked[i] = rand.nextU();

[tfarina, 2014/01/08 18:32:40]

ok, you put random numbers in an array of 32 elements.

[bungeman-skia, 2014/01/09 16:08:33]

The magic of putting this all on one line is almost too much to resist. But I
think this is a bit easier to read like

for (...) {
    ...body...
}

or maybe like

for (size_t i = 0; i < SK_ARRAY_COUNT(tweaked); data[i] = tweaked[i] =
rand.nextU(), ++i);

haha, I kid.

[mtklein, 2014/01/09 16:21:46]

Done.

+
+    // Test each algorithm.
+    for (size_t i = 0; i < SK_ARRAY_COUNT(kAlgorithms); i++) {

[tfarina, 2014/01/08 18:32:40]

then you iterate through the array of function pointers, to test
SkChecksum::Compute and SkChecksum::Murmur3

[bungeman-skia, 2014/01/09 16:08:33]

Totally ocd, but the post increment makes me wince. I like ++i for all those
reasons that don't matter here.

[mtklein, 2014/01/09 16:21:46]

Done.

+        // Hash of NULL is always 0.
+        ASSERT(kAlgorithms[i](NULL, 0) == 0);

[bungeman-skia, 2014/01/09 16:08:33]

There's a lot of kAlgorithms[i] going on here, can we get a

algorithmProc& currentAlgorithm = kAlgorithms[i];

and use that instead? Makes things a bit clearer.

[mtklein, 2014/01/09 16:21:46]

Done.

+
+        const uint32_t hash = kAlgorithms[i](data, kBytes);

[tfarina, 2014/01/08 18:32:40]

so you call the algorithm to hash the data and store it to...

+        // Should be deterministic.
+        ASSERT(hash == kAlgorithms[i](data, kBytes));

[tfarina, 2014/01/08 18:32:40]

make sure it will always return the same hash for the same data you pass.

+
+        // Changing any one element should change the hash.

[tfarina, 2014/01/08 18:32:40]

"any one element", that is hard to translate to my native language, Portuguese.
could you find a better word?

[mtklein, 2014/01/08 18:41:55]

"any single element"?  The idea is that we have [ n0 n1 n2 n3 ... n31 ] in our
array.  We want to make sure that our hash function is sensitive enough to
changes that changing any of those 32 n's to a different value, even just one,
changes the hash.

This is a simple approximation of a full avalanche test,
http://pt.wikipedia.org/wiki/Efeito_avalanche_(criptografia) .

+        for (size_t j = 0; j < SK_ARRAY_COUNT(tweaked); j++) {
+            const uint32_t saved = tweaked[j];
+            tweaked[j] = rand.nextU();

[tfarina, 2014/01/08 18:32:40]

ok, so here you are changing the data you have previous stored in the original
array, hence you named the array 'tweaked'.

+            const uint32_t tweakedHash = kAlgorithms[i](tweaked, kBytes);

[tfarina, 2014/01/08 18:32:40]

then you call the algorithm again, to obtain a new hash.

+            ASSERT(tweakedHash != hash);

[tfarina, 2014/01/08 18:32:40]

and it should be different for the original hash, because you changed the value
of the elements of the array 'tweaked'.

+            ASSERT(tweakedHash == kAlgorithms[i](tweaked, kBytes));

[tfarina, 2014/01/08 18:32:40]

then you hash again to make sure the algorithm is deterministic.

+            tweaked[j] = saved;

[tfarina, 2014/01/08 18:32:40]

and restore the original value (that part I didn't follow, is it necessary?).

[mtklein, 2014/01/08 18:41:55]

Not strictly necessary, but we want to make sure we're only looking at one
changed number at a time.  This loop checks each location independently.  If we
left out tweaked[j] = saved;, we'd be testing
  j = 0: is the hash different if we change one number?
  j = 1: is the hash different if we change two numbers?
  j = 2: is the hash different if we change three numbers?
  etc.

+        }
+    }
 }