Improve testing of acquireEnumGuard() by checking that guard is taken on underlying target attribute.

1 files changed, 12 insertions, 7 deletions

diff --git a/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp b/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp
index 62b337bfc59..108def1b829 100644
--- a/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp
+++ b/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp
@@ -12,10 +12,11 @@
 #include <vespa/searchlib/test/mock_gid_to_lid_mapping.h>
 #include <vespa/searchcommon/attribute/attributecontent.h>
 #include <vespa/vespalib/testkit/testapp.h>
-#include <memory>
+#include <algorithm>
 #include <map>
+#include <memory>
+#include <thread>
 #include <vector>
-#include <algorithm>
 
 namespace search {
 namespace attribute {
@@ -223,6 +224,13 @@ void reset_with_wset_value_reference_mappings(
     f.reset_with_wset_value_reference_mappings<AttrVecType, WeightedValueType>(type, mappings);
 }
 
+bool has_active_enum_guards(AttributeVector &attr) {
+    bool result;
+    std::thread thread([&result, &attr]() { result = attr.hasActiveEnumGuards(); });
+    thread.join();
+    return result;
+}
+
 TEST_F("Accessors return expected attributes", Fixture) {
     EXPECT_EQUAL(f.imported_attr->getReferenceAttribute().get(),
                  f.reference_attr.get());
@@ -285,11 +293,6 @@ TEST_F("acquireEnumGuard() acquires enum guard on target and regular guard on re
     add_n_docs_with_undefined_values(*f.target_attr, 2);
     {
         auto guard = f.imported_attr->acquireEnumGuard();
-        // We can add docs, but not mutate enum values. That would deadlock the test, as the
-        // attribute enum guard already holds a shared lock. Could test the mutex itself to
-        // verify that it is indeed locked, but that's a private member in AttributeVector,
-        // so would be Very Dirty(tm).
-        // TODO find a robust way to test this.
         add_n_docs_with_undefined_values(*f.target_attr, 1);
         add_n_docs_with_undefined_values(*f.reference_attr, 1);
 
@@ -298,12 +301,14 @@ TEST_F("acquireEnumGuard() acquires enum guard on target and regular guard on re
 
         EXPECT_EQUAL(3u, f.target_attr->getFirstUsedGeneration());
         EXPECT_EQUAL(1u, f.reference_attr->getFirstUsedGeneration());
+        EXPECT_TRUE(has_active_enum_guards(*f.target_attr));
     }
     // Force a generation handler update
     add_n_docs_with_undefined_values(*f.reference_attr, 1);
     add_n_docs_with_undefined_values(*f.target_attr, 1);
     EXPECT_EQUAL(7u, f.target_attr->getFirstUsedGeneration());
     EXPECT_EQUAL(3u, f.reference_attr->getFirstUsedGeneration());
+    EXPECT_FALSE(has_active_enum_guards(*f.target_attr));
 }
 
 TEST_F("Single-valued integer attribute values can be retrieved via reference", Fixture) {