Test closest feature setup failures.

3 files changed, 62 insertions, 16 deletions

diff --git a/searchlib/src/tests/features/closest/closest_test.cpp b/searchlib/src/tests/features/closest/closest_test.cpp
index 3fd366f4253..c53e627b528 100644
--- a/searchlib/src/tests/features/closest/closest_test.cpp
+++ b/searchlib/src/tests/features/closest/closest_test.cpp
@@ -11,7 +11,7 @@
 #include <vespa/searchlib/test/features/distance_closeness_fixture.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <vespa/vespalib/gtest/gtest.h>
-#include <vespa/vespalib/util/stringfmt.h>
+#include <vespa/vespalib/stllike/asciistream.h>
 
 using search::feature_t;
 using search::features::test::BlueprintFactoryFixture;
@@ -64,6 +64,30 @@ std::ostream& operator<<(std::ostream& os, const TestParam param)
     return os;
 }
 
+void
+assert_setup(vespalib::string field_name,
+             bool exp_setup_result,
+             std::optional<vespalib::string> attr_type_spec,
+             std::optional<vespalib::string> label)
+{
+    vespalib::asciistream feature_name;
+    std::vector<vespalib::string> setup_args;
+    ClosestBlueprint f1;
+    IndexEnvironmentFixture f2;
+    DummyDependencyHandler deps(f1);
+    setup_args.emplace_back(field_name);
+    feature_name << f1.getBaseName() << "(" << field_name;
+    if (label.has_value()) {
+        feature_name << "," << label.value();
+        setup_args.emplace_back(label.value());
+    }
+    feature_name << ")";
+    f1.setName(feature_name.str());
+    if (attr_type_spec.has_value()) {
+        search::fef::indexproperties::type::Attribute::set(f2.indexEnv.getProperties(), field_name, attr_type_spec.value());
+    }
+    EXPECT_EQ(exp_setup_result, static_cast<Blueprint&>(f1).setup(f2.indexEnv, setup_args));
+}
 
 class ClosestTest : public ::testing::TestWithParam<TestParam>
 {
@@ -122,6 +146,41 @@ TEST(ClosestTest, require_that_no_features_are_dumped)
     f1.visitDumpFeatures(f2.indexEnv, f3);
 }
 
+TEST(ClosestTest, require_that_setup_fails_for_unknown_field)
+{
+    assert_setup("random_field", false, mixed_tensor_type, std::nullopt);
+}
+
+TEST(ClosestTest, require_that_setup_fails_if_field_type_is_not_attribute)
+{
+    assert_setup("ibar", false, sparse_tensor_type, std::nullopt);
+}
+
+TEST(ClosestTest, require_that_setup_fails_if_field_data_type_is_not_tensor)
+{
+    assert_setup("foo", false, sparse_tensor_type, std::nullopt);
+}
+
+TEST(ClosestTest, require_that_setup_can_be_done_on_random_label)
+{
+    assert_setup("bar", true, mixed_tensor_type, "random_label");
+}
+
+TEST(ClosestTest, require_that_setup_fails_if_tensor_type_is_missing)
+{
+    assert_setup("bar", false, std::nullopt, std::nullopt);
+}
+
+TEST(ClosestTest, require_that_setup_fails_if_tensor_type_is_dense)
+{
+    assert_setup("bar", false, dense_tensor_type, std::nullopt);
+}
+
+TEST(ClosestTest, require_that_setup_fails_if_tensor_type_is_sparse)
+{
+    assert_setup("bar", false, sparse_tensor_type, std::nullopt);
+}
+
 TEST_P(ClosestTest, require_that_no_label_gives_empty_result)
 {
     NoLabel f;
diff --git a/searchlib/src/vespa/searchlib/features/closest_feature.cpp b/searchlib/src/vespa/searchlib/features/closest_feature.cpp
index 62a9ce0ba24..c0284c9fa89 100644
--- a/searchlib/src/vespa/searchlib/features/closest_feature.cpp
+++ b/searchlib/src/vespa/searchlib/features/closest_feature.cpp
@@ -231,21 +231,7 @@ ClosestBlueprint::setup(const fef::IIndexEnvironment & env, const fef::Parameter
         _item_label = params[1].getValue();
     }
     auto fi = env.getFieldByName(_field_name);
-    if (fi == nullptr) {
-        LOG(error, "%s: Unknown field %s", getName().c_str(), _field_name.c_str());
-        return false;
-    }
-    auto dt = fi->get_data_type();
-    auto ct = fi->collection();
-    if (dt != search::index::schema::DataType::TENSOR ||
-        ct != search::index::schema::CollectionType::SINGLE) {
-        LOG(error, "%s: Field %s is not a single value tensor field", getName().c_str(), _field_name.c_str());
-        return false;
-    }
-    if (!fi->hasAttribute()) {
-        LOG(error, "%s: Field %s does not have an attribute", getName().c_str(), _field_name.c_str());
-        return false;
-    }
+    assert(fi != nullptr);
     vespalib::string attr_type_spec = type::Attribute::lookup(env.getProperties(), _field_name);
     if (attr_type_spec.empty()) {
         LOG(error, "%s: Field %s lacks a type in index properties", getName().c_str(), _field_name.c_str());
diff --git a/searchlib/src/vespa/searchlib/test/features/distance_closeness_fixture.h b/searchlib/src/vespa/searchlib/test/features/distance_closeness_fixture.h
index 02879b8f0e3..8aae1ecb942 100644
--- a/searchlib/src/vespa/searchlib/test/features/distance_closeness_fixture.h
+++ b/searchlib/src/vespa/searchlib/test/features/distance_closeness_fixture.h
@@ -33,6 +33,7 @@ struct IndexEnvironmentFixture {
         IndexEnvironmentBuilder builder(indexEnv);
         builder.addField(FieldType::ATTRIBUTE, FieldInfo::CollectionType::SINGLE, FieldInfo::DataType::INT64, "foo");
         builder.addField(FieldType::ATTRIBUTE, FieldInfo::CollectionType::SINGLE, FieldInfo::DataType::TENSOR, "bar");
+        builder.addField(FieldType::INDEX, FieldInfo::CollectionType::SINGLE, FieldInfo::DataType::TENSOR, "ibar");
     }
 };