From 5bc1c438effa9f226b4155da48257bd00a605010 Mon Sep 17 00:00:00 2001 From: kortschak Date: Mon, 7 Jul 2014 12:01:16 +0930 Subject: [PATCH 1/3] Tabulate value comparison tests --- graph/iterator/value_comparison_iterator_test.go | 143 +++++++++++++---------- 1 file changed, 80 insertions(+), 63 deletions(-) diff --git a/graph/iterator/value_comparison_iterator_test.go b/graph/iterator/value_comparison_iterator_test.go index 94555d7..2c605ec 100644 --- a/graph/iterator/value_comparison_iterator_test.go +++ b/graph/iterator/value_comparison_iterator_test.go @@ -15,6 +15,7 @@ package iterator import ( + "reflect" "testing" "github.com/google/cayley/graph" @@ -30,78 +31,94 @@ func simpleFixedIterator() *Fixed { return f } -func checkIteratorContains(ts graph.TripleStore, it graph.Iterator, expected []string, t *testing.T) { - var actual []string - actual = nil - for { - val, ok := it.Next() - if !ok { - break - } - actual = append(actual, ts.NameOf(val)) - } - actualSet := actual[:] - for _, a := range expected { - found := false - for j, b := range actualSet { - if a == b { - actualSet = append(actualSet[:j], actualSet[j+1:]...) - found = true +var comparisonTests = []struct { + message string + operand graph.Value + operator Operator + expect []string +}{ + { + message: "successful int64 less than comparison", + operand: int64(3), + operator: kCompareLT, + expect: []string{"0", "1", "2"}, + }, + { + message: "empty int64 less than comparison", + operand: int64(0), + operator: kCompareLT, + expect: nil, + }, + { + message: "successful int64 greater than comparison", + operand: int64(2), + operator: kCompareGT, + expect: []string{"3", "4"}, + }, + { + message: "successful int64 greater than or equal comparison", + operand: int64(2), + operator: kCompareGTE, + expect: []string{"2", "3", "4"}, + }, +} + +func TestValueComparison(t *testing.T) { + for _, test := range comparisonTests { + ts := simpleStore + vc := NewComparison(simpleFixedIterator(), test.operator, test.operand, ts) + + var got []string + for { + val, ok := vc.Next() + if !ok { break } + got = append(got, ts.NameOf(val)) } - if !found { - t.Error("Couldn't find", a, "in actual output.\nActual:", actual, "\nExpected: ", expected, "\nRemainder: ", actualSet) - return + if !reflect.DeepEqual(got, test.expect) { + t.Errorf("Failed to show %s, got:%q expect:%q", test.message, got, test.expect) } } - if len(actualSet) != 0 { - t.Error("Actual output has more than expected.\nActual:", actual, "\nExpected: ", expected, "\nRemainder: ", actualSet) - } } -func TestWorkingIntValueComparison(t *testing.T) { - ts := simpleStore - fixed := simpleFixedIterator() - vc := NewComparison(fixed, kCompareLT, int64(3), ts) - checkIteratorContains(ts, vc, []string{"0", "1", "2"}, t) -} - -func TestFailingIntValueComparison(t *testing.T) { - ts := simpleStore - fixed := simpleFixedIterator() - vc := NewComparison(fixed, kCompareLT, int64(0), ts) - checkIteratorContains(ts, vc, []string{}, t) -} - -func TestWorkingGT(t *testing.T) { - ts := simpleStore - fixed := simpleFixedIterator() - vc := NewComparison(fixed, kCompareGT, int64(2), ts) - checkIteratorContains(ts, vc, []string{"3", "4"}, t) -} - -func TestWorkingGTE(t *testing.T) { - ts := simpleStore - fixed := simpleFixedIterator() - vc := NewComparison(fixed, kCompareGTE, int64(2), ts) - checkIteratorContains(ts, vc, []string{"2", "3", "4"}, t) +var vciCheckTests = []struct { + message string + operator Operator + check graph.Value + expect bool +}{ + { + message: "1 is less than 2", + operator: kCompareGTE, + check: 1, + expect: false, + }, + { + message: "2 is greater than or equal to 2", + operator: kCompareGTE, + check: 2, + expect: true, + }, + { + message: "3 is greater than or equal to 2", + operator: kCompareGTE, + check: 3, + expect: true, + }, + { + message: "5 is absent from iterator", + operator: kCompareGTE, + check: 5, + expect: false, + }, } func TestVCICheck(t *testing.T) { - ts := simpleStore - fixed := simpleFixedIterator() - vc := NewComparison(fixed, kCompareGTE, int64(2), ts) - if vc.Check(1) { - t.Error("1 is less than 2, should be GTE") - } - if !vc.Check(2) { - t.Error("2 is GTE 2") - } - if !vc.Check(3) { - t.Error("3 is GTE 2") - } - if vc.Check(5) { - t.Error("5 is not in the underlying iterator") + for _, test := range vciCheckTests { + vc := NewComparison(simpleFixedIterator(), test.operator, int64(2), simpleStore) + if vc.Check(test.check) != test.expect { + t.Errorf("Failed to show %s", test.message) + } } } From d1fdba1cbb482cd22a1ee86b26c0db09d7b411bd Mon Sep 17 00:00:00 2001 From: kortschak Date: Mon, 7 Jul 2014 12:14:47 +0930 Subject: [PATCH 2/3] Fix build leveldb did s/GetApproximateSizes/SizeOf/ --- graph/leveldb/triplestore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/leveldb/triplestore.go b/graph/leveldb/triplestore.go index 427722a..1b4de48 100644 --- a/graph/leveldb/triplestore.go +++ b/graph/leveldb/triplestore.go @@ -394,7 +394,7 @@ func (ts *TripleStore) GetApproximateSizeForPrefix(pre []byte) (int64, error) { ranges := make([]util.Range, 1) ranges[0].Start = pre ranges[0].Limit = limit - sizes, err := ts.db.GetApproximateSizes(ranges) + sizes, err := ts.db.SizeOf(ranges) if err == nil { return (int64(sizes[0]) >> 6) + 1, nil } From e39063e3eccba507a10fa5f8cb8eff068e0598cc Mon Sep 17 00:00:00 2001 From: kortschak Date: Mon, 7 Jul 2014 12:23:32 +0930 Subject: [PATCH 3/3] Rename some methods and funcs since we are here Very probably some of these can be made private. --- graph/leveldb/all_iterator.go | 2 +- graph/leveldb/iterator.go | 6 +++--- graph/leveldb/leveldb_test.go | 4 ++-- graph/leveldb/triplestore.go | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index 9036769..88a1386 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -105,7 +105,7 @@ func (it *AllIterator) Close() { } func (it *AllIterator) Size() (int64, bool) { - size, err := it.ts.GetApproximateSizeForPrefix(it.prefix) + size, err := it.ts.SizeOfPrefix(it.prefix) if err == nil { return size, false } diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index 8e80e7b..be044e9 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -114,7 +114,7 @@ func (it *Iterator) Next() (graph.Value, bool) { return nil, false } -func GetPositionFromPrefix(prefix []byte, d graph.Direction, ts *TripleStore) int { +func PositionOf(prefix []byte, d graph.Direction, ts *TripleStore) int { if bytes.Equal(prefix, []byte("sp")) { switch d { case graph.Subject: @@ -171,7 +171,7 @@ func (it *Iterator) Check(v graph.Value) bool { if val[0] == 'z' { return false } - offset := GetPositionFromPrefix(val[0:2], it.dir, it.ts) + offset := PositionOf(val[0:2], it.dir, it.ts) if offset != -1 { if bytes.HasPrefix(val[offset:], it.checkId[1:]) { return true @@ -187,7 +187,7 @@ func (it *Iterator) Check(v graph.Value) bool { } func (it *Iterator) Size() (int64, bool) { - return it.ts.GetSizeFor(it.checkId), true + return it.ts.SizeOf(it.checkId), true } func (it *Iterator) DebugString(indent int) string { diff --git a/graph/leveldb/leveldb_test.go b/graph/leveldb/leveldb_test.go index 57821a1..3e50122 100644 --- a/graph/leveldb/leveldb_test.go +++ b/graph/leveldb/leveldb_test.go @@ -176,7 +176,7 @@ func TestLoadDatabase(t *testing.T) { if s := ts.Size(); s != 11 { t.Errorf("Unexpected triplestore size, got:%d expect:11", s) } - if s := ts.GetSizeFor(ts.ValueOf("B")); s != 5 { + if s := ts.SizeOf(ts.ValueOf("B")); s != 5 { t.Errorf("Unexpected triplestore size, got:%d expect:5", s) } @@ -184,7 +184,7 @@ func TestLoadDatabase(t *testing.T) { if s := ts.Size(); s != 10 { t.Errorf("Unexpected triplestore size after RemoveTriple, got:%d expect:10", s) } - if s := ts.GetSizeFor(ts.ValueOf("B")); s != 4 { + if s := ts.SizeOf(ts.ValueOf("B")); s != 4 { t.Errorf("Unexpected triplestore size, got:%d expect:4", s) } diff --git a/graph/leveldb/triplestore.go b/graph/leveldb/triplestore.go index 1b4de48..7880165 100644 --- a/graph/leveldb/triplestore.go +++ b/graph/leveldb/triplestore.go @@ -332,7 +332,7 @@ func (ts *TripleStore) ValueOf(s string) graph.Value { return ts.createValueKeyFor(s) } -func (ts *TripleStore) getValueData(value_key []byte) ValueData { +func (ts *TripleStore) valueData(value_key []byte) ValueData { var out ValueData if glog.V(3) { glog.V(3).Infof("%s %v\n", string(value_key[0]), value_key) @@ -357,14 +357,14 @@ func (ts *TripleStore) NameOf(k graph.Value) string { glog.V(2).Infoln("k was nil") return "" } - return ts.getValueData(k.([]byte)).Name + return ts.valueData(k.([]byte)).Name } -func (ts *TripleStore) GetSizeFor(k graph.Value) int64 { +func (ts *TripleStore) SizeOf(k graph.Value) int64 { if k == nil { return 0 } - return int64(ts.getValueData(k.([]byte)).Size) + return int64(ts.valueData(k.([]byte)).Size) } func (ts *TripleStore) getSize() { @@ -386,7 +386,7 @@ func (ts *TripleStore) getSize() { ts.size = size } -func (ts *TripleStore) GetApproximateSizeForPrefix(pre []byte) (int64, error) { +func (ts *TripleStore) SizeOfPrefix(pre []byte) (int64, error) { limit := make([]byte, len(pre)) copy(limit, pre) end := len(limit) - 1 @@ -428,7 +428,7 @@ func (ts *TripleStore) TriplesAllIterator() graph.Iterator { func (ts *TripleStore) TripleDirection(val graph.Value, d graph.Direction) graph.Value { v := val.([]uint8) - offset := GetPositionFromPrefix(v[0:2], d, ts) + offset := PositionOf(v[0:2], d, ts) if offset != -1 { return append([]byte("z"), v[offset:offset+ts.hasher.Size()]...) } else {