CodeGen: simplify unit tests [2/3]

CodeGen: simplify unit tests [2/3]

Most of CodeGen's unit tests cover internal algorithmic details that
are about to go away, so remove them all except for the one truly
interesting test: the graph implied from the original
MakeInstruction() calls should match the graph implied from the
resulting BPF program.

BUG=414363
Committed: https://crrev.com/9681fa57fcb814f0cacee84cecd4e751ce33553a
Cr-Commit-Position: refs/heads/master@{#304525}

[mdempsky, 2014-11-04 00:56:12]

mdempsky@chromium.org changed reviewers:
+ jln@chromium.org, rickyz@chromium.org

[rickyz (no longer on Chrome), 2014-11-05 23:36:57]

lgtm

https://codereview.chromium.org/694423003/diff/20001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/codegen_unittest.cc (right):

https://codereview.chromium.org/694423003/diff/20001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:166: return
addr_hashes_.find(next)->second;
Maybe assert that the entry is present?

[mdempsky, 2014-11-06 02:42:43]

https://codereview.chromium.org/694423003/diff/20001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/codegen_unittest.cc (right):

https://codereview.chromium.org/694423003/diff/20001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:166: return
addr_hashes_.find(next)->second;
On 2014/11/05 23:36:57, rickyz wrote:
> Maybe assert that the entry is present?

Done.  (Has to be EXPECT because of non-void return; but in debug builds,
dereferencing an invalid iterator assert fails anyway.)

[jln (very slow on Chromium), 2014-11-14 04:54:29]

lgtm

https://chromiumcodereview.appspot.com/694423003/diff/40001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf/codegen_unittest.cc (right):

https://chromiumcodereview.appspot.com/694423003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:45: Hash(const Hash& hash) :
digest_(hash.digest_) {}
= default instead?

https://chromiumcodereview.appspot.com/694423003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:46: Hash& operator=(const Hash&
rhs) {
= default?

Otherwise, why not use copy-and-swap instead?

https://chromiumcodereview.appspot.com/694423003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:52: return
std::memcmp(&lhs.digest_, &rhs.digest_, sizeof(lhs.digest_)) == 0;
:( Too bad they didn't provide operator==() in base/

You could use MD5DigestToBase16 and compare that maybe?

https://chromiumcodereview.appspot.com/694423003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:168: return it->second;
It feels wrong to cause a crash here if EXPECT_NE triggers. In practice this
would be silent (we would only see a crash).

Maybe turn it into a if with a EXPECT_TRUE(false) or something along these
lines?

[mdempsky, 2014-11-17 23:00:05]

https://codereview.chromium.org/694423003/diff/40001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/codegen_unittest.cc (right):

https://codereview.chromium.org/694423003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:45: Hash(const Hash& hash) :
digest_(hash.digest_) {}
On 2014/11/14 04:54:29, jln (Tokyo til 14th) wrote:
> = default instead?

Done.  (Wrote this code while "= default" was still banned and forgot to update
it; thanks!)

https://codereview.chromium.org/694423003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:46: Hash& operator=(const Hash&
rhs) {
On 2014/11/14 04:54:29, jln (Tokyo til 14th) wrote:
> = default?

Done.

https://codereview.chromium.org/694423003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:52: return
std::memcmp(&lhs.digest_, &rhs.digest_, sizeof(lhs.digest_)) == 0;
On 2014/11/14 04:54:29, jln (Tokyo til 14th) wrote:
> :( Too bad they didn't provide operator==() in base/
> 
> You could use MD5DigestToBase16 and compare that maybe?

Done.

https://codereview.chromium.org/694423003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/codegen_unittest.cc:168: return it->second;
On 2014/11/14 04:54:29, jln (Tokyo til 14th) wrote:
> It feels wrong to cause a crash here if EXPECT_NE triggers. In practice this
> would be silent (we would only see a crash).
> 
> Maybe turn it into a if with a EXPECT_TRUE(false) or something along these
> lines?

Done.

[mdempsky, 2014-11-17 23:28:32]

The CQ bit was checked by mdempsky@chromium.org

[commit-bot: I haz the power, 2014-11-17 23:30:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/694423003/60001

[commit-bot: I haz the power, 2014-11-18 00:22:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-11-18 00:23:00]

Patchset 4 (id:??) landed as
https://crrev.com/9681fa57fcb814f0cacee84cecd4e751ce33553a
Cr-Commit-Position: refs/heads/master@{#304525}