Stress tests #54
Stress tests #54
Conversation
- added benchmarks for creating large directory and doing lookups from large directory - TB test - MakeRand and Digest test
- s/createLargePadForBenchmark/createPad <- could be useful for other tests, too - added tests for some edge cases - removed obsolete check `uint64 < 0` which can't happen and error
- benchmark pad.Update(nil) in tree with 10^5 users (takes on average 0.23 s and consumes 72 MB)
- more stress tests/benchmarks for PAD Update and LookUp (used for pprof)
- test coverage in utils package
| if int(binary.LittleEndian.Uint32(b)) != numInt { | ||
| t.Fatal("Conversion to bytes looks wrong!") | ||
| } | ||
| // TODO It is possible to call `IntToBytes` with a signed int < 0 |
There was a problem hiding this comment.
Continue the discussion in #43 (comment).
Do you have any explanation for this? I really don't know the reason why Golang developers do this. Even if I try int(4294967254) == -42, it still returns false.
There was a problem hiding this comment.
I thought this will be discussed (and resolved) in #53 ?
| } | ||
| numInt = -42 | ||
| b = IntToBytes(numInt) | ||
| if int32(binary.LittleEndian.Uint32(b)) != int32(numInt) { |
There was a problem hiding this comment.
Can it be simplified to int32(binary.LittleEndian.Uint32(b)) != numInt?
There was a problem hiding this comment.
Wait, I changed another line; but here it's needed or not? int32(binary.LittleEndian.Uint32(b)) != numInt won't work because the type of numInt is an int and the type of int32(binary.LittleEndian.Uint32(b)) is an int32
There was a problem hiding this comment.
You're right. Sorry for the noise.
| // create next epoch and see if the TB is inserted as promised: | ||
| pad.Update(nil) | ||
|
|
||
| ap, err := pad.Lookup(key1) |
There was a problem hiding this comment.
Should we also verify the proof of inclusion here, just to make this test as a sample for complete TB verification?
There was a problem hiding this comment.
Yup, I suggest to make some small changes, though.
There was a problem hiding this comment.
Thanks for your feedback :-) 95650ab
| if !bytes.Equal(ap.LookupIndex, tb.Index) || !bytes.Equal(ap.Leaf.Value(), tb.Value) { | ||
| t.Error("Value wasn't inserted as promised") | ||
| } | ||
| // step 1. verify VRF index | ||
| if !bytes.Equal(vrfPrivKey1.Compute([]byte(key)), ap.LookupIndex) { |
There was a problem hiding this comment.
I think we should verify the VRF index in returned TB first (after verifying the signature), then we could remove this verification since we already compared both indices above.
|
LGTM. Nice work @liamsi :). Thanks! |
| @@ -241,14 +241,14 @@ func TestPoliciesChange(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestTB(t *testing.T) { | |||
There was a problem hiding this comment.
Should TestTB() be moved to tb_test.go?
There was a problem hiding this comment.
Yes, why not. It makes sense (04cf99a)
|
LGTM. Thanks @liamsi! |
(stress) tests and some (test-related) improvements: - added benchmarks for creating large directory, doing lookups from large directory, and calling `PAD.Update(nil)` - added tests: MakeRand, Digest, and TB test - added tests for some edge cases - test coverage in utils package and some edge cases in other packages - removed obsolete check `uint64 < 0` which can't happen (and corresponding error)
Reopening for discussion here since I messed up #43 .