Extended feature validations in REST tests and improved wording. (#108292)

server/src/main/java/org/elasticsearch/features/FeatureSpecification.java

@@ -26,6 +26,10 @@ import java.util.Set;
  * All feature checks should be done through {@code FeatureService} to ensure that Elasticsearch's
  * guarantees on the introduction of new functionality are followed;
  * that is, new functionality is not enabled until all nodes in the cluster support it.
+ * <p>
+ * <b>Note:</b> {@link FeatureSpecification}s are loaded as service providers, however tests are not fully modularized yet.
+ * Make sure to also register new specifications in {@code META-INF/services/org.elasticsearch.features.FeatureSpecification},
+ * so they are available in tests as well.
  */
 public interface FeatureSpecification {
     /**

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -350,7 +350,13 @@ public abstract class ESRestTestCase extends ESTestCase {
         assert nodesVersions != null;
     }
 
-    protected List<FeatureSpecification> createAdditionalFeatureSpecifications() {
+    /**
+     * Override to provide additional test-only historical features.
+     *
+     * Note: This extension point cannot be used to add cluster features. The provided {@link FeatureSpecification}s
+     * must contain only historical features, otherwise an assertion error is thrown.
+     */
+    protected List<FeatureSpecification> additionalTestOnlyHistoricalFeatures() {
         return List.of();
     }
 
@@ -368,7 +374,7 @@ public abstract class ESRestTestCase extends ESTestCase {
             );
         }
         return new ESRestTestFeatureService(
-            createAdditionalFeatureSpecifications(),
+            additionalTestOnlyHistoricalFeatures(),
             semanticNodeVersions,
             ClusterFeatures.calculateAllNodeFeatures(clusterStateFeatures.values())
         );

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestFeatureService.java

@@ -58,10 +58,15 @@ class ESRestTestFeatureService implements TestFeatureService {
         if (MetadataHolder.HISTORICAL_FEATURES != null) {
             specs.add(MetadataHolder.HISTORICAL_FEATURES);
         }
-        var historicalFeatures = FeatureData.createFromSpecifications(specs).getHistoricalFeatures();
-        this.knownHistoricalFeatureNames = historicalFeatures.lastEntry().getValue();
+        FeatureData featureData = FeatureData.createFromSpecifications(specs);
+        assert featureData.getNodeFeatures().isEmpty()
+            : Strings.format(
+                "Only historical features can be injected via ESRestTestCase#additionalTestOnlyHistoricalFeatures(), rejecting %s",
+                featureData.getNodeFeatures().keySet()
+            );
+        this.knownHistoricalFeatureNames = featureData.getHistoricalFeatures().lastEntry().getValue();
         this.version = nodeVersions.stream().min(Comparator.naturalOrder()).orElse(Version.CURRENT);
-        this.allSupportedFeatures = Sets.union(clusterStateFeatures, historicalFeatures.floorEntry(version).getValue());
+        this.allSupportedFeatures = Sets.union(clusterStateFeatures, featureData.getHistoricalFeatures().floorEntry(version).getValue());
     }
 
     public static boolean hasFeatureMetadata() {
@@ -88,8 +93,8 @@ class ESRestTestFeatureService implements TestFeatureService {
                 throw new IllegalArgumentException(
                     Strings.format(
                         "Synthetic version features are only available before [%s] for migration purposes! "
-                            + "Please add a cluster feature to an appropriate FeatureSpecification; features only necessary for "
-                            + "testing can be supplied via ESRestTestCase#createAdditionalFeatureSpecifications()",
+                            + "Please add a cluster feature to an appropriate FeatureSpecification; test-only historical-features  "
+                            + "can be supplied via ESRestTestCase#additionalTestOnlyHistoricalFeatures()",
                         Version.V_8_15_0
                     )
                 );
@@ -100,10 +105,9 @@ class ESRestTestFeatureService implements TestFeatureService {
         if (hasFeatureMetadata()) {
             throw new IllegalArgumentException(
                 Strings.format(
-                    "Unknown feature %s: check the feature has been added to the correct FeatureSpecification in the relevant module or, "
-                        + "if this is a legacy feature used only in tests, to a test-only FeatureSpecification such as %s.",
-                    featureId,
-                    RestTestLegacyFeatures.class.getCanonicalName()
+                    "Unknown feature %s: check the respective FeatureSpecification is provided both in module-info.java "
+                        + "as well as in META-INF/services and verify the module is loaded during tests.",
+                    featureId
                 )
             );
         }