Merge pull request #13728 from vespa-engine/toregge/extend-document-weight-or-filter-search-unit-test

Extend unit test for document weight or filter search.

8 files changed, 93 insertions, 2 deletions

diff --git a/searchlib/src/tests/attribute/document_weight_or_filter_search/CMakeLists.txt b/searchlib/src/tests/attribute/document_weight_or_filter_search/CMakeLists.txt
index 660cfbaefff..8f25f6b66b8 100644
--- a/searchlib/src/tests/attribute/document_weight_or_filter_search/CMakeLists.txt
+++ b/searchlib/src/tests/attribute/document_weight_or_filter_search/CMakeLists.txt
@@ -5,6 +5,7 @@ vespa_add_executable(searchlib_document_weight_or_filter_search_test_app TEST
     document_weight_or_filter_search_test.cpp
     DEPENDS
     searchlib
+    searchlib_test
     GTest::GTest
 )
 vespa_add_test(NAME searchlib_document_weight_or_filter_search_test_app COMMAND searchlib_document_weight_or_filter_search_test_app)
diff --git a/searchlib/src/tests/attribute/document_weight_or_filter_search/document_weight_or_filter_search_test.cpp b/searchlib/src/tests/attribute/document_weight_or_filter_search/document_weight_or_filter_search_test.cpp
index c8e799a8f10..22f928ddf45 100644
--- a/searchlib/src/tests/attribute/document_weight_or_filter_search/document_weight_or_filter_search_test.cpp
+++ b/searchlib/src/tests/attribute/document_weight_or_filter_search/document_weight_or_filter_search_test.cpp
@@ -5,6 +5,8 @@
 #include <vespa/searchlib/attribute/document_weight_or_filter_search.h>
 #include <vespa/searchlib/queryeval/searchiterator.h>
 #include <vespa/searchlib/common/bitvector.h>
+#define ENABLE_GTEST_MIGRATION
+#include <vespa/searchlib/test/searchiteratorverifier.h>
 
 using PostingList = search::attribute::PostingListTraits<int32_t>::PostingStoreBase;
 using Iterator = search::attribute::PostingListTraits<int32_t>::const_iterator;
@@ -20,7 +22,7 @@ class DocumentWeightOrFilterSearchTest : public ::testing::Test {
     std::vector<EntryRef> _trees;
     uint32_t _range_start;
     uint32_t _range_end;
-protected:
+public:
     DocumentWeightOrFilterSearchTest();
     ~DocumentWeightOrFilterSearchTest();
     void inc_generation();
@@ -48,6 +50,13 @@ protected:
         _postings.apply(_trees[idx], &*adds.begin(), &*adds.end(), &*removes.begin(), &*removes.end());
     }
 
+    void clear_tree(size_t idx) {
+        if (idx < _trees.size()) {
+            _postings.clear(_trees[idx]);
+            _trees[idx] = EntryRef();
+        }
+    }
+
     std::unique_ptr<SearchIterator> make_iterator() const {
         std::vector<Iterator> iterators;
         for (size_t i = 0; i < num_trees(); ++i) {
@@ -200,4 +209,53 @@ TEST_F(DocumentWeightOrFilterSearchTest, taat_and_hits_into_ranged)
     expect_result(frombv(*bv), { 14 });
 }
 
+namespace {
+
+class Verifier : public search::test::SearchIteratorVerifier {
+    DocumentWeightOrFilterSearchTest &_test;
+public:
+    Verifier(DocumentWeightOrFilterSearchTest &test, int num_trees)
+        : _test(test)
+    {
+        std::vector<std::vector<uint32_t>> trees(num_trees);
+        uint32_t tree_id = 0;
+        for (const auto doc_id : getExpectedDocIds()) {
+            trees[tree_id++ % trees.size()].emplace_back(doc_id);
+        }
+        tree_id = 0;
+        for (const auto &tree : trees) {
+            _test.add_tree(tree_id++, tree);
+        }
+        _test.inc_generation();
+    }
+    ~Verifier() {
+        for (uint32_t tree_id = 0; tree_id < _test.num_trees(); ++tree_id) {
+            _test.clear_tree(tree_id);
+        }
+        _test.inc_generation();
+    }
+    std::unique_ptr<SearchIterator> create(bool) const override {
+        return _test.make_iterator();
+    }
+
+};
+
+TEST_F(DocumentWeightOrFilterSearchTest, iterator_conformance)
+{
+    {
+        Verifier verifier(*this, 1);
+        verifier.verify();
+    }
+    {
+        Verifier verifier(*this, 2);
+        verifier.verify();
+    }
+    {
+        Verifier verifier(*this, 3);
+        verifier.verify();
+    }
+}
+
+}
+
 GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/searchlib/src/vespa/searchlib/attribute/document_weight_or_filter_search.cpp b/searchlib/src/vespa/searchlib/attribute/document_weight_or_filter_search.cpp
index 35c5ce75f33..d9134417d27 100644
--- a/searchlib/src/vespa/searchlib/attribute/document_weight_or_filter_search.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/document_weight_or_filter_search.cpp
@@ -53,7 +53,7 @@ DocumentWeightOrFilterSearchImpl::doSeek(uint32_t docId)
         _children.seek(0, docId);
     }
     uint32_t min_doc_id = _children.get_docid(0);
-    for (uint16_t i = 1; min_doc_id > docId && i < _children.size(); ++i) {
+    for (uint16_t i = 1; i < _children.size(); ++i) {
         if (_children.get_docid(i) < docId) {
             _children.seek(i, docId);
         }
diff --git a/searchlib/src/vespa/searchlib/test/CMakeLists.txt b/searchlib/src/vespa/searchlib/test/CMakeLists.txt
index 41084148c87..4d722e687ef 100644
--- a/searchlib/src/vespa/searchlib/test/CMakeLists.txt
+++ b/searchlib/src/vespa/searchlib/test/CMakeLists.txt
@@ -1,4 +1,5 @@
 # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+find_package(GTest REQUIRED)
 vespa_add_library(searchlib_test
     SOURCES
     document_weight_attribute_helper.cpp
@@ -12,7 +13,17 @@ vespa_add_library(searchlib_test
     statestring.cpp
     $<TARGET_OBJECTS:searchlib_test_fakedata>
     $<TARGET_OBJECTS:searchlib_searchlib_test_diskindex>
+    $<TARGET_OBJECTS:searchlib_test_gtest_migration>
     DEPENDS
     searchlib
     searchlib_searchlib_test_memoryindex
+    GTest::GTest
 )
+
+vespa_add_library(searchlib_test_gtest_migration OBJECT
+    SOURCES
+    initrange.cpp
+    searchiteratorverifier.cpp
+)
+
+target_compile_definitions(searchlib_test_gtest_migration PRIVATE ENABLE_GTEST_MIGRATION)
diff --git a/searchlib/src/vespa/searchlib/test/initrange.cpp b/searchlib/src/vespa/searchlib/test/initrange.cpp
index 2292a8e775e..1e23f7c5b8c 100644
--- a/searchlib/src/vespa/searchlib/test/initrange.cpp
+++ b/searchlib/src/vespa/searchlib/test/initrange.cpp
@@ -1,6 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #include "initrange.h"
+#ifdef ENABLE_GTEST_MIGRATION
+#include <vespa/vespalib/gtest/gtest.h>
+#define ASSERT_EQUAL ASSERT_EQ
+#define EXPECT_EQUAL EXPECT_EQ
+#else
 #include <vespa/vespalib/testkit/test_kit.h>
+#endif
 #include <vespa/searchlib/queryeval/emptysearch.h>
 #include <vespa/searchlib/queryeval/truesearch.h>
 #include <algorithm>
diff --git a/searchlib/src/vespa/searchlib/test/initrange.h b/searchlib/src/vespa/searchlib/test/initrange.h
index a143dfdb119..4fbb851779a 100644
--- a/searchlib/src/vespa/searchlib/test/initrange.h
+++ b/searchlib/src/vespa/searchlib/test/initrange.h
@@ -7,6 +7,10 @@
 
 namespace search::test {
 
+#ifdef ENABLE_GTEST_MIGRATION
+#define InitRangeVerifier InitRangeVerifierForGTest
+#endif
+
 class InitRangeVerifier {
 public:
     typedef queryeval::SearchIterator SearchIterator;
diff --git a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp
index ec53d6d9d00..276f8fbc08d 100644
--- a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp
+++ b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp
@@ -2,7 +2,14 @@
 
 #include "searchiteratorverifier.h"
 #include "initrange.h"
+#ifdef ENABLE_GTEST_MIGRATION
+#include <vespa/vespalib/gtest/gtest.h>
+#define TEST_DO(x) x
+#define EXPECT_EQUAL EXPECT_EQ
+#define ASSERT_EQUAL ASSERT_EQ
+#else
 #include <vespa/vespalib/testkit/test_kit.h>
+#endif
 #include <vespa/searchlib/queryeval/emptysearch.h>
 #include <vespa/searchlib/queryeval/truesearch.h>
 #include <vespa/searchlib/queryeval/termwise_search.h>
diff --git a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.h b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.h
index 3d35731dab1..afeb46f0c16 100644
--- a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.h
+++ b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.h
@@ -7,6 +7,10 @@
 
 namespace search::test {
 
+#ifdef ENABLE_GTEST_MIGRATION
+#define SearchIteratorVerifier SearchIteratorVerifierForGTest
+#endif
+
 class SearchIteratorVerifier {
 public:
     typedef queryeval::SearchIterator SearchIterator;