Merge pull request #11019 from vespa-engine/vekterli/avoid-wset-ordering-dependencies-in-tests

Avoid dependency on weighted set ordering in unit tests

6 files changed, 141 insertions, 43 deletions

diff --git a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp
index 78cd9ce44b9..9b930e271bd 100644
--- a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp
+++ b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp
@@ -29,6 +29,7 @@
 #include <vespa/searchlib/attribute/reference_attribute.h>
 #include <vespa/searchlib/tensor/dense_tensor_attribute.h>
 #include <vespa/searchlib/tensor/generic_tensor_attribute.h>
+#include <vespa/searchlib/test/weighted_type_test_utils.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
 #include <vespa/vespalib/testkit/testapp.h>
 
@@ -158,9 +159,14 @@ check(const AttributePtr &vec, uint32_t docId, const std::vector<T> &values)
     std::vector<T> buf(sz);
     uint32_t asz = vec->get(docId, &buf[0], sz);
     if (!EXPECT_EQUAL(sz, asz)) return false;
+    std::vector<T> wanted(values.begin(), values.end());
+    if (vec->hasWeightedSetType()) {
+        std::sort(wanted.begin(), wanted.end(), value_then_weight_order());
+        std::sort(buf.begin(), buf.end(), value_then_weight_order());
+    }
     for (uint32_t i = 0; i < values.size(); ++i) {
-        if (!EXPECT_EQUAL(buf[i].getValue(), values[i].getValue())) return false;
-        if (!EXPECT_EQUAL(buf[i].getWeight(), values[i].getWeight())) return false;
+        if (!EXPECT_EQUAL(buf[i].getValue(), wanted[i].getValue())) return false;
+        if (!EXPECT_EQUAL(buf[i].getWeight(), wanted[i].getWeight())) return false;
     }
     return true;
 }
diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp
index 2f3df28cc6f..ce1344f5c26 100644
--- a/searchlib/src/tests/attribute/attribute_test.cpp
+++ b/searchlib/src/tests/attribute/attribute_test.cpp
@@ -19,6 +19,7 @@
 #include <vespa/searchlib/attribute/singlestringattribute.h>
 #include <vespa/searchlib/index/dummyfileheadercontext.h>
 #include <vespa/searchlib/util/randomgenerator.h>
+#include <vespa/searchlib/test/weighted_type_test_utils.h>
 #include <vespa/vespalib/io/fileutil.h>
 #include <vespa/vespalib/testkit/testapp.h>
 #include <cmath>
@@ -137,6 +138,20 @@ replace_suffix(AttributeVector &v, const vespalib::string &suffix)
     return name + suffix;
 }
 
+template <typename Container, typename V>
+bool contains(const Container& c, size_t elems, const V& value) {
+    auto end = c.begin() + elems;
+    return (std::find(c.begin(), end, value) != end);
+}
+
+template <typename Container, typename V>
+bool contains_value(const Container& c, size_t elems, const V& value) {
+    auto end = c.begin() + elems;
+    return (std::find_if(c.begin(), end, [&value](const auto& ws_elem) {
+        return (ws_elem.getValue() == value);
+    }) != end);
+}
+
 }
 
 namespace search {
@@ -411,7 +426,13 @@ void AttributeTest::compare(VectorType & a, VectorType & b)
         ASSERT_TRUE(a.getValueCount(i) == b.getValueCount(i));
         EXPECT_EQUAL(static_cast<const AttributeVector &>(a).get(i, av, asz), static_cast<uint32_t>(a.getValueCount(i)));
         EXPECT_EQUAL(static_cast<const AttributeVector &>(b).get(i, bv, bsz), static_cast<uint32_t>(b.getValueCount(i)));
-        for(size_t j(0), k(std::min(a.getValueCount(i), b.getValueCount(i))); j < k; j++) {
+        const size_t min_common_value_count = std::min(a.getValueCount(i), b.getValueCount(i));
+        if (a.hasWeightedSetType()) {
+            ASSERT_TRUE(b.hasWeightedSetType());
+            std::sort(av, av + min_common_value_count, order_by_value());
+            std::sort(bv, bv + min_common_value_count, order_by_value());
+        }
+        for(size_t j = 0; j < min_common_value_count; j++) {
             EXPECT_EQUAL(av[j], bv[j]);
         }
     }
@@ -1076,6 +1097,9 @@ AttributeTest::testArray()
 // CollectionType::WSET
 //-----------------------------------------------------------------------------
 
+// This function makes the assumption that weights are unique, so that is has a way
+// of creating a deterministic comparison ordering of weighted sets without caring about
+// the templated values themselvecs.
 template <typename VectorType, typename BufferType>
 void
 AttributeTest::testWeightedSet(const AttributePtr & ptr, const std::vector<BufferType> & values)
@@ -1089,6 +1113,9 @@ AttributeTest::testWeightedSet(const AttributePtr & ptr, const std::vector<Buffe
     uint32_t bufferSize = numDocs + 10;
     std::vector<BufferType> buffer(bufferSize);
 
+    std::vector<BufferType> ordered_values(values.begin(), values.end());
+    std::sort(ordered_values.begin(), ordered_values.end(), order_by_weight());
+
     // fill and check
     EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 0u);
     EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u);
@@ -1099,10 +1126,11 @@ AttributeTest::testWeightedSet(const AttributePtr & ptr, const std::vector<Buffe
             EXPECT_TRUE(v.append(doc, values[j].getValue(), values[j].getWeight()));
         }
         commit(ptr);
-        EXPECT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount);
+        ASSERT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount);
+        std::sort(buffer.begin(), buffer.begin() + valueCount, order_by_weight());
         for (uint32_t j = 0; j < valueCount; ++j) {
-            EXPECT_TRUE(buffer[j].getValue() == values[j].getValue());
-            EXPECT_TRUE(buffer[j].getWeight() == values[j].getWeight());
+            EXPECT_TRUE(buffer[j].getValue() == ordered_values[j].getValue());
+            EXPECT_TRUE(buffer[j].getWeight() == ordered_values[j].getWeight());
         }
     }
     EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), numDocs + (numDocs*(numDocs-1))/2);
@@ -1115,24 +1143,21 @@ AttributeTest::testWeightedSet(const AttributePtr & ptr, const std::vector<Buffe
         // append non-existent value
         EXPECT_TRUE(v.append(doc, values[doc].getValue(), values[doc].getWeight()));
         commit(ptr);
-        EXPECT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 1);
-        EXPECT_TRUE(buffer[doc].getValue() == values[doc].getValue());
-        EXPECT_TRUE(buffer[doc].getWeight() == values[doc].getWeight());
+        ASSERT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 1);
+        EXPECT_TRUE(contains(buffer, valueCount + 1, values[doc]));
 
         // append existent value
         EXPECT_TRUE(v.append(doc, values[doc].getValue(), values[doc].getWeight() + 10));
         commit(ptr);
-        EXPECT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 1);
-        EXPECT_TRUE(buffer[doc].getValue() == values[doc].getValue());
-        EXPECT_TRUE(buffer[doc].getWeight() == values[doc].getWeight() + 10);
+        ASSERT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 1);
+        EXPECT_TRUE(contains(buffer, valueCount + 1, BufferType(values[doc].getValue(), values[doc].getWeight() + 10)));
 
         // append non-existent value two times
         EXPECT_TRUE(v.append(doc, values[doc + 1].getValue(), values[doc + 1].getWeight()));
         EXPECT_TRUE(v.append(doc, values[doc + 1].getValue(), values[doc + 1].getWeight() + 10));
         commit(ptr);
-        EXPECT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 2);
-        EXPECT_TRUE(buffer[doc + 1].getValue() == values[doc + 1].getValue());
-        EXPECT_TRUE(buffer[doc + 1].getWeight() == values[doc + 1].getWeight() + 10);
+        ASSERT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 2);
+        EXPECT_TRUE(contains(buffer, valueCount + 2, BufferType(values[doc + 1].getValue(), values[doc + 1].getWeight() + 10)));
     }
     EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), numDocs + (numDocs*(numDocs-1))/2 + numDocs*4);
     EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u);
@@ -1148,14 +1173,12 @@ AttributeTest::testWeightedSet(const AttributePtr & ptr, const std::vector<Buffe
         EXPECT_TRUE(static_cast<uint32_t>(v.getValueCount(doc)) == valueCount + 2);
 
         // remove existent value
-        EXPECT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 2);
-        EXPECT_TRUE(buffer[doc + 1].getValue() == values[doc + 1].getValue());
+        ASSERT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 2);
+        EXPECT_TRUE(contains_value(buffer, valueCount + 2, values[doc + 1].getValue()));
         EXPECT_TRUE(v.remove(doc, values[doc + 1].getValue(), 0));
         commit(ptr);
-        EXPECT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 1);
-        for (uint32_t i = 0; i < valueCount + 1; ++i) {
-            EXPECT_TRUE(buffer[i].getValue() != values[doc + 1].getValue());
-        }
+        ASSERT_TRUE(ptr->get(doc, &buffer[0], buffer.size()) == valueCount + 1);
+        EXPECT_FALSE(contains_value(buffer, valueCount + 1, values[doc + 1].getValue()));
     }
     EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), numDocs + (numDocs*(numDocs-1))/2 + numDocs*4 + numDocs * 2);
     EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u);
@@ -1512,8 +1535,9 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue
     ptr->commit();
     if (createIfNonExistant) {
         EXPECT_EQUAL(ptr->get(5, &buf[0], 2), uint32_t(2));
-        EXPECT_EQUAL(buf[0].getWeight(), 100);
-        EXPECT_EQUAL(buf[1].getWeight(), 10);
+        std::sort(buf.begin(), buf.begin() + 2, order_by_weight());
+        EXPECT_EQUAL(buf[0].getWeight(), 10);
+        EXPECT_EQUAL(buf[1].getWeight(), 100);
     } else {
         EXPECT_EQUAL(ptr->get(5, &buf[0], 2), uint32_t(1));
         EXPECT_EQUAL(buf[0].getWeight(), 100);
diff --git a/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp b/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp
index e30b41f477d..31af5945337 100644
--- a/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp
+++ b/searchlib/src/tests/attribute/enum_attribute_compaction/enum_attribute_compaction_test.cpp
@@ -6,6 +6,7 @@
 #include <vespa/searchlib/attribute/attributevector.hpp>
 #include <vespa/searchlib/attribute/integerbase.h>
 #include <vespa/searchlib/attribute/stringbase.h>
+#include <vespa/searchlib/test/weighted_type_test_utils.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP("enum_attribute_compaction_test");
@@ -141,8 +142,13 @@ CompactionTest<VectorType>::check_values(uint32_t doc_id)
     buffer.fill(*_v, doc_id);
     if (_v->hasMultiValue()) {
         EXPECT_EQ(2u, buffer.size());
-        EXPECT_EQ(CheckType(buffer[0]), MyTestData::make_value(doc_id, 0));
-        EXPECT_EQ(CheckType(buffer[1]), MyTestData::make_value(doc_id, 1));
+        int i = 0, j = 1;
+        if (_v->hasWeightedSetType() && !(CheckType(buffer[0]) == MyTestData::make_value(doc_id, 0))) {
+            i = 1;
+            j = 0;
+        }
+        EXPECT_EQ(CheckType(buffer[i]), MyTestData::make_value(doc_id, 0));
+        EXPECT_EQ(CheckType(buffer[j]), MyTestData::make_value(doc_id, 1));
     } else {
         EXPECT_EQ(1u, buffer.size());
         EXPECT_EQ(CheckType(buffer[0]), MyTestData::make_value(doc_id, 0));
diff --git a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp
index 5c73a60f6ef..a9c6cd1fd6c 100644
--- a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp
+++ b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp
@@ -105,6 +105,22 @@ StringAttributeTest::testMultiValue()
 
 }
 
+namespace {
+
+template <typename T0, typename T1>
+auto zipped_and_sorted_by_first(const std::vector<T0>& a, const std::vector<T1>& b) -> std::vector<std::pair<T0, T1>> {
+    std::vector<std::pair<T0, T1>> combined;
+    assert(a.size() == b.size());
+    for (size_t i = 0; i < a.size(); ++i) {
+        combined.emplace_back(a[i], b[i]);
+    }
+    std::sort(combined.begin(), combined.end(), [](const auto& lhs, const auto& rhs){
+        return (lhs.first < rhs.first);
+    });
+    return combined;
+}
+
+}
 
 template <typename Attribute>
 void
@@ -118,14 +134,16 @@ StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs)
     for (uint32_t i = 0; i < numDocs - 1; ++i) {
         char unique[16];
         sprintf(unique, i < 10 ? "enum0%u" : "enum%u", i);
-        uniqueStrings.push_back(vespalib::string(unique));
+        uniqueStrings.emplace_back(unique);
     }
+    ASSERT_TRUE(std::is_sorted(uniqueStrings.begin(), uniqueStrings.end()));
+
     std::vector<vespalib::string> newUniques;
     newUniques.reserve(numDocs - 1);
     for (uint32_t i = 0; i < numDocs - 1; ++i) {
         char unique[16];
         sprintf(unique, i < 10 ? "unique0%u" : "unique%u", i);
-        newUniques.push_back(vespalib::string(unique));
+        newUniques.emplace_back(unique);
     }
 
     // add docs
@@ -147,10 +165,10 @@ StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs)
 
         // test get first
         if (valueCount == 0) {
-            EXPECT_TRUE(attr.get(doc) == NULL);
+            EXPECT_TRUE(attr.get(doc) == nullptr);
             EXPECT_TRUE(attr.getEnum(doc) == std::numeric_limits<uint32_t>::max());
-        } else {
-            EXPECT_TRUE(strcmp(attr.get(doc), uniqueStrings[0].c_str()) == 0);
+        } else if (!attr.hasWeightedSetType()) {
+            EXPECT_EQUAL(vespalib::string(attr.get(doc)), uniqueStrings[0]);
             uint32_t e;
             EXPECT_TRUE(attr.findEnum(uniqueStrings[0].c_str(), e));
             EXPECT_EQUAL(1u, attr.findFoldedEnums(uniqueStrings[0].c_str()).size());
@@ -160,17 +178,17 @@ StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs)
 
         // test get all
         std::vector<vespalib::string> values(valueCount);
-        EXPECT_TRUE(attr.get(doc, &values[0], valueCount) == valueCount);
+        ASSERT_TRUE(attr.get(doc, &values[0], valueCount) == valueCount);
 
         std::vector<uint32_t> enums(valueCount);
-        EXPECT_TRUE((static_cast<search::attribute::IAttributeVector &>(attr)).get(doc, &enums[0], valueCount) == valueCount);
+        ASSERT_TRUE((static_cast<search::attribute::IAttributeVector &>(attr)).get(doc, &enums[0], valueCount) == valueCount);
 
+        auto combined = zipped_and_sorted_by_first(values, enums);
         for (uint32_t j = 0; j < valueCount; ++j) {
-            //LOG(info, "doc[%u][%u] = %s", doc, j, values[j].c_str());
-            EXPECT_TRUE(values[j] == uniqueStrings[j]);
+            EXPECT_TRUE(combined[j].first == uniqueStrings[j]);
             uint32_t e = 100;
-            EXPECT_TRUE(attr.findEnum(values[j].c_str(), e));
-            EXPECT_TRUE(enums[j] == e);
+            EXPECT_TRUE(attr.findEnum(combined[j].first.c_str(), e));
+            EXPECT_TRUE(combined[j].second == e);
         }
     }
 
@@ -207,12 +225,12 @@ StringAttributeTest::testMultiValue(Attribute & attr, uint32_t numDocs)
         std::vector<uint32_t> enums(valueCount);
         EXPECT_TRUE((static_cast<search::attribute::IAttributeVector &>(attr)).get(doc, &enums[0], valueCount) == valueCount);
 
+        auto combined = zipped_and_sorted_by_first(values, enums);
         for (uint32_t j = 0; j < valueCount; ++j) {
-            //LOG(info, "doc[%u][%u] = %s", doc, j, values[j].c_str());
-            EXPECT_TRUE(values[j] == newUniques[j]);
+            EXPECT_TRUE(combined[j].first == newUniques[j]);
             uint32_t e = 100;
-            EXPECT_TRUE(attr.findEnum(values[j].c_str(), e));
-            EXPECT_TRUE(enums[j] == e);
+            EXPECT_TRUE(attr.findEnum(combined[j].first.c_str(), e));
+            EXPECT_TRUE(combined[j].second == e);
         }
     }
 
diff --git a/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h b/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h
index c37cfc38a8c..a58f71d232b 100644
--- a/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h
+++ b/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h
@@ -3,6 +3,7 @@
 #pragma once
 
 #include "mock_gid_to_lid_mapping.h"
+#include "weighted_type_test_utils.h"
 #include <vespa/document/base/documentid.h>
 #include <vespa/document/base/globalid.h>
 #include <vespa/searchlib/attribute/attribute_read_guard.h>
@@ -296,7 +297,7 @@ ImportedAttributeFixture::ImportedAttributeFixture(bool use_search_cache_)
     reference_attr->setGidToLidMapperFactory(mapper_factory);
 }
 
-ImportedAttributeFixture::~ImportedAttributeFixture() {}
+ImportedAttributeFixture::~ImportedAttributeFixture() = default;
 
 template<typename AttrValueType, typename PredicateType>
 void assert_multi_value_matches(const ImportedAttributeFixture &f,
@@ -305,9 +306,14 @@ void assert_multi_value_matches(const ImportedAttributeFixture &f,
                                 PredicateType predicate) {
     AttributeContent<AttrValueType> content;
     content.fill(*f.get_imported_attr(), lid);
-    EXPECT_EQUAL(expected.size(), content.size());
+    ASSERT_EQUAL(expected.size(), content.size());
     std::vector<AttrValueType> actual(content.begin(), content.end());
-    EXPECT_TRUE(std::equal(expected.begin(), expected.end(),
+    std::vector<AttrValueType> wanted(expected.begin(), expected.end());
+    if constexpr (IsWeightedType<AttrValueType>::value) {
+        std::sort(actual.begin(), actual.end(), value_then_weight_order());
+        std::sort(wanted.begin(), wanted.end(), value_then_weight_order());
+    }
+    EXPECT_TRUE(std::equal(wanted.begin(), wanted.end(),
                            actual.begin(), actual.end(), predicate));
 }
 
diff --git a/searchlib/src/vespa/searchlib/test/weighted_type_test_utils.h b/searchlib/src/vespa/searchlib/test/weighted_type_test_utils.h
new file mode 100644
index 00000000000..ac9fefbfb35
--- /dev/null
+++ b/searchlib/src/vespa/searchlib/test/weighted_type_test_utils.h
@@ -0,0 +1,38 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include <vespa/searchcommon/attribute/iattributevector.h>
+#include <type_traits>
+
+template <typename T> struct IsWeightedType : std::false_type {};
+template <typename T> struct IsWeightedType<search::attribute::WeightedType<T>> : std::true_type {};
+
+struct value_then_weight_order {
+    template <typename T>
+    bool operator()(const T& lhs, const T& rhs) const noexcept {
+        if (lhs.getValue() != rhs.getValue()) {
+            return (lhs.getValue() < rhs.getValue());
+        }
+        return (lhs.getWeight() < rhs.getWeight());
+    }
+};
+
+struct order_by_value {
+    template <typename T>
+    bool operator()(const T& lhs, const T& rhs) const noexcept {
+        if constexpr (IsWeightedType<T>::value) {
+            return (lhs.getValue() < rhs.getValue());
+        } else {
+            return (lhs < rhs);
+        }
+    }
+};
+
+
+struct order_by_weight {
+    template <typename T>
+    bool operator()(const search::attribute::WeightedType<T>& lhs,
+                    const search::attribute::WeightedType<T>& rhs) const noexcept {
+        return (lhs.getWeight() < rhs.getWeight());
+    }
+};