Merge pull request #13607 from vespa-engine/geirst/refactor-attribute-writer-tests

Refactor attribute writer tests

3 files changed, 58 insertions, 43 deletions

diff --git a/searchcore/src/tests/proton/attribute/CMakeLists.txt b/searchcore/src/tests/proton/attribute/CMakeLists.txt
index 1f385628480..c23d97c6e88 100644
--- a/searchcore/src/tests/proton/attribute/CMakeLists.txt
+++ b/searchcore/src/tests/proton/attribute/CMakeLists.txt
@@ -7,6 +7,7 @@ vespa_add_executable(searchcore_attribute_test_app TEST
     searchcore_attribute
     searchcore_flushengine
     searchcore_pcommon
+    searchlib_test
     gtest
 )
 vespa_add_test(NAME searchcore_attribute_test_app COMMAND searchcore_attribute_test_app)
diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp
index 6284fe119c4..0c21fabc27a 100644
--- a/searchcore/src/tests/proton/attribute/attribute_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp
@@ -19,6 +19,7 @@
 #include <vespa/searchcore/proton/attribute/imported_attributes_repo.h>
 #include <vespa/searchcore/proton/common/hw_info.h>
 #include <vespa/searchcore/proton/test/attribute_utils.h>
+#include <vespa/searchcore/proton/test/mock_attribute_manager.h>
 #include <vespa/searchcorespi/flush/iflushtarget.h>
 #include <vespa/searchlib/attribute/attribute_read_guard.h>
 #include <vespa/searchlib/attribute/attributefactory.h>
@@ -57,6 +58,7 @@ using namespace vespa::config::search;
 
 using proton::ImportedAttributesRepo;
 using proton::test::AttributeUtils;
+using proton::test::MockAttributeManager;
 using search::TuneFileAttributes;
 using search::attribute::BitVectorSearchCache;
 using search::attribute::IAttributeVector;
@@ -118,20 +120,16 @@ const std::shared_ptr<IDestructorCallback> emptyCallback;
 class AttributeWriterTest : public ::testing::Test {
 public:
     DirectoryHandler _dirHandler;
-    DummyFileHeaderContext _fileHeaderContext;
     std::unique_ptr<ForegroundTaskExecutor> _attributeFieldWriterReal;
     std::unique_ptr<SequencedTaskExecutorObserver> _attributeFieldWriter;
-    HwInfo _hwInfo;
-    proton::AttributeManager::SP _m;
+    std::shared_ptr<MockAttributeManager> _mgr;
     std::unique_ptr<AttributeWriter> _aw;
 
     AttributeWriterTest()
         : _dirHandler(test_dir),
-          _fileHeaderContext(),
           _attributeFieldWriterReal(),
           _attributeFieldWriter(),
-          _hwInfo(),
-          _m(),
+          _mgr(),
           _aw()
     {
         setup(1);
@@ -141,18 +139,19 @@ public:
         _aw.reset();
         _attributeFieldWriterReal = std::make_unique<ForegroundTaskExecutor>(threads);
         _attributeFieldWriter = std::make_unique<SequencedTaskExecutorObserver>(*_attributeFieldWriterReal);
-        _m = std::make_shared<proton::AttributeManager>(test_dir, "test.subdb", TuneFileAttributes(),
-                                                        _fileHeaderContext, *_attributeFieldWriter, _hwInfo);
+        _mgr = std::make_shared<MockAttributeManager>();
+        _mgr->set_writer(*_attributeFieldWriter);
         allocAttributeWriter();
     }
     void allocAttributeWriter() {
-        _aw = std::make_unique<AttributeWriter>(_m);
+        _aw = std::make_unique<AttributeWriter>(_mgr);
     }
     AttributeVector::SP addAttribute(const vespalib::string &name) {
-        return addAttribute({name, AVConfig(AVBasicType::INT32)}, createSerialNum);
+        return addAttribute({name, AVConfig(AVBasicType::INT32)});
     }
-    AttributeVector::SP addAttribute(const AttributeSpec &spec, SerialNum serialNum) {
-        auto ret = _m->addAttribute(spec, serialNum);
+    AttributeVector::SP addAttribute(const AttributeSpec &spec) {
+        auto ret = _mgr->addAttribute(spec.getName(),
+                                    AttributeFactory::createAttribute(spec.getName(), spec.getConfig()));
         allocAttributeWriter();
         return ret;
     }
@@ -195,9 +194,9 @@ TEST_F(AttributeWriterTest, handles_put)
     DocBuilder idb(s);
 
     auto a1 = addAttribute("a1");
-    auto a2 = addAttribute({"a2", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)}, createSerialNum);
-    auto a3 = addAttribute({"a3", AVConfig(AVBasicType::FLOAT)}, createSerialNum);
-    auto a4 = addAttribute({"a4", AVConfig(AVBasicType::STRING)}, createSerialNum);
+    auto a2 = addAttribute({"a2", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)});
+    auto a3 = addAttribute({"a3", AVConfig(AVBasicType::FLOAT)});
+    auto a4 = addAttribute({"a4", AVConfig(AVBasicType::STRING)});
 
     attribute::IntegerContent ibuf;
     attribute::FloatContent fbuf;
@@ -275,7 +274,7 @@ TEST_F(AttributeWriterTest, handles_predicate_put)
     s.addAttributeField(Schema::AttributeField("a1", schema::DataType::BOOLEANTREE, CollectionType::SINGLE));
     DocBuilder idb(s);
 
-    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::PREDICATE)}, createSerialNum);
+    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::PREDICATE)});
 
     PredicateIndex &index = static_cast<PredicateAttribute &>(*a1).getIndex();
 
@@ -369,7 +368,7 @@ verifyAttributeContent(const AttributeVector & v, uint32_t lid, vespalib::string
 
 TEST_F(AttributeWriterTest, visibility_delay_is_honoured)
 {
-    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::STRING)}, createSerialNum);
+    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::STRING)});
     Schema s;
     s.addAttributeField(Schema::AttributeField("a1", schema::DataType::STRING, CollectionType::SINGLE));
     DocBuilder idb(s);
@@ -381,7 +380,7 @@ TEST_F(AttributeWriterTest, visibility_delay_is_honoured)
     put(3, *doc, 1);
     EXPECT_EQ(2u, a1->getNumDocs());
     EXPECT_EQ(3u, a1->getStatus().getLastSyncToken());
-    AttributeWriter awDelayed(_m);
+    AttributeWriter awDelayed(_mgr);
     awDelayed.put(4, *doc, 2, false, emptyCallback);
     EXPECT_EQ(3u, a1->getNumDocs());
     EXPECT_EQ(3u, a1->getStatus().getLastSyncToken());
@@ -391,7 +390,7 @@ TEST_F(AttributeWriterTest, visibility_delay_is_honoured)
     awDelayed.forceCommit(6, emptyCallback);
     EXPECT_EQ(6u, a1->getStatus().getLastSyncToken());
 
-    AttributeWriter awDelayedShort(_m);
+    AttributeWriter awDelayedShort(_mgr);
     awDelayedShort.put(7, *doc, 2, false, emptyCallback);
     EXPECT_EQ(6u, a1->getStatus().getLastSyncToken());
     awDelayedShort.put(8, *doc, 2, false, emptyCallback);
@@ -414,7 +413,7 @@ TEST_F(AttributeWriterTest, visibility_delay_is_honoured)
 
 TEST_F(AttributeWriterTest, handles_predicate_remove)
 {
-    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::PREDICATE)}, createSerialNum);
+    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::PREDICATE)});
     Schema s;
     s.addAttributeField(
             Schema::AttributeField("a1", schema::DataType::BOOLEANTREE, CollectionType::SINGLE));
@@ -477,7 +476,7 @@ TEST_F(AttributeWriterTest, handles_update)
 
 TEST_F(AttributeWriterTest, handles_predicate_update)
 {
-    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::PREDICATE)}, createSerialNum);
+    auto a1 = addAttribute({"a1", AVConfig(AVBasicType::PREDICATE)});
     Schema schema;
     schema.addAttributeField(Schema::AttributeField("a1", schema::DataType::BOOLEANTREE, CollectionType::SINGLE));
 
@@ -628,7 +627,7 @@ AttributeVector::SP
 createTensorAttribute(AttributeWriterTest &t) {
     AVConfig cfg(AVBasicType::TENSOR);
     cfg.setTensorType(ValueType::from_spec("tensor(x{},y{})"));
-    auto ret = t.addAttribute({"a1", cfg}, createSerialNum);
+    auto ret = t.addAttribute({"a1", cfg});
     return ret;
 }
 
@@ -724,9 +723,9 @@ putAttributes(AttributeWriterTest &t, std::vector<uint32_t> expExecuteHistory)
 
     DocBuilder idb(s);
 
-    AttributeVector::SP a1 = t.addAttribute("a1");
-    AttributeVector::SP a2 = t.addAttribute("a2");
-    AttributeVector::SP a3 = t.addAttribute("a3");
+    auto a1 = t.addAttribute("a1");
+    auto a2 = t.addAttribute("a2");
+    auto a3 = t.addAttribute("a3");
 
     EXPECT_EQ(1u, a1->getNumDocs());
     EXPECT_EQ(1u, a2->getNumDocs());
@@ -785,10 +784,10 @@ createImportedAttributesRepo()
 
 TEST_F(AttributeWriterTest, forceCommit_clears_search_cache_in_imported_attribute_vectors)
 {
-    _m->setImportedAttributes(createImportedAttributesRepo());
+    _mgr->setImportedAttributes(createImportedAttributesRepo());
     commit(10);
-    EXPECT_EQ(0u, _m->getImportedAttributes()->get("imported_a")->getSearchCache()->size());
-    EXPECT_EQ(0u, _m->getImportedAttributes()->get("imported_b")->getSearchCache()->size());
+    EXPECT_EQ(0u, _mgr->getImportedAttributes()->get("imported_a")->getSearchCache()->size());
+    EXPECT_EQ(0u, _mgr->getImportedAttributes()->get("imported_b")->getSearchCache()->size());
 }
 
 class StructWriterTestBase : public AttributeWriterTest {
@@ -803,7 +802,7 @@ public:
           _valueField("value", 2, *DataType::INT, true),
           _structFieldType("struct")
     {
-        addAttribute({"value", AVConfig(AVBasicType::INT32, AVCollectionType::SINGLE)}, createSerialNum);
+        addAttribute({"value", AVConfig(AVBasicType::INT32, AVCollectionType::SINGLE)});
         _type.addField(_valueField);
         _structFieldType.addField(_valueField);
     }
@@ -837,7 +836,7 @@ public:
           _structArrayFieldType(_structFieldType),
           _structArrayField("array", _structArrayFieldType, true)
     {
-        addAttribute({"array.value", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)}, createSerialNum);
+        addAttribute({"array.value", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)});
         _type.addField(_structArrayField);
     }
     ~StructArrayWriterTest();
@@ -853,8 +852,8 @@ public:
         return doc;
     }
     void checkAttrs(uint32_t lid, int32_t value, const std::vector<int32_t> &arrayValues) {
-        auto valueAttr = _m->getAttribute("value")->getSP();
-        auto arrayValueAttr = _m->getAttribute("array.value")->getSP();
+        auto valueAttr = _mgr->getAttribute("value")->getSP();
+        auto arrayValueAttr = _mgr->getAttribute("array.value")->getSP();
         EXPECT_EQ(value, valueAttr->getInt(lid));
         attribute::IntegerContent ibuf;
         ibuf.fill(*arrayValueAttr, lid);
@@ -888,8 +887,8 @@ public:
           _structMapFieldType(*DataType::INT, _structFieldType),
           _structMapField("map", _structMapFieldType, true)
     {
-        addAttribute({"map.value.value", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)}, createSerialNum);
-        addAttribute({"map.key", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)}, createSerialNum);
+        addAttribute({"map.value.value", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)});
+        addAttribute({"map.key", AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY)});
         _type.addField(_structMapField);
     }
 
@@ -905,9 +904,9 @@ public:
     }
 
     void checkAttrs(uint32_t lid, int32_t expValue, const std::map<int32_t, int32_t> &expMap) {
-        auto valueAttr = _m->getAttribute("value")->getSP();
-        auto mapKeyAttr = _m->getAttribute("map.key")->getSP();
-        auto mapValueAttr = _m->getAttribute("map.value.value")->getSP();
+        auto valueAttr = _mgr->getAttribute("value")->getSP();
+        auto mapKeyAttr = _mgr->getAttribute("map.key")->getSP();
+        auto mapValueAttr = _mgr->getAttribute("map.value.value")->getSP();
         EXPECT_EQ(expValue, valueAttr->getInt(lid));
         attribute::IntegerContent mapKeys;
         mapKeys.fill(*mapKeyAttr, lid);
diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h
index 3e49bb449ff..8e5d3018532 100644
--- a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h
+++ b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h
@@ -11,16 +11,26 @@ namespace proton::test {
 class MockAttributeManager : public IAttributeManager {
 private:
     search::attribute::test::MockAttributeManager _mock;
+    std::vector<search::AttributeVector*> _writables;
     std::unique_ptr<ImportedAttributesRepo> _importedAttributes;
+    vespalib::ISequencedTaskExecutor* _writer;
 
 public:
     MockAttributeManager()
         : _mock(),
-          _importedAttributes()
+          _writables(),
+          _importedAttributes(),
+          _writer()
     {}
 
-    void addAttribute(const vespalib::string &name, const search::AttributeVector::SP &attr) {
+    search::AttributeVector::SP addAttribute(const vespalib::string &name, const search::AttributeVector::SP &attr) {
         _mock.addAttribute(name, attr);
+        _writables.push_back(attr.get());
+        return attr;
+    }
+
+    void set_writer(vespalib::ISequencedTaskExecutor& writer) {
+        _writer = &writer;
     }
 
     search::AttributeGuard::UP getAttribute(const vespalib::string &name) const override {
@@ -56,13 +66,18 @@ public:
         HDR_ABORT("should not be reached");
     }
     vespalib::ISequencedTaskExecutor &getAttributeFieldWriter() const override {
-        HDR_ABORT("should not be reached");
-    }
-    search::AttributeVector *getWritableAttribute(const vespalib::string &) const override {
+        assert(_writer != nullptr);
+        return *_writer;
+    }
+    search::AttributeVector *getWritableAttribute(const vespalib::string &name) const override {
+        auto attr = getAttribute(name);
+        if (attr) {
+            return attr->get();
+        }
         return nullptr;
     }
     const std::vector<search::AttributeVector *> &getWritableAttributes() const override {
-        HDR_ABORT("should not be reached");
+        return _writables;
     }
     void asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor>) const override {
     }