ESQL: Fix EsqlNodeSubclassTests (elastic#116496) (elastic#116505)

- Fix EsqlNodeSubclassTests so that they also test the Node subclasses in esql.core. - Fix the re-enabled tests for Field-/Reference-/MetadataAttribute by using the longest public c'tor in their info() method implementations.
mark-vieira · Nov 8, 2024 · 8321277 · 8321277
1 parent 7460360
commit 8321277
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 64 deletions.
diff --git a/.../esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java b/.../esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java
@@ -174,18 +174,7 @@ public String getWriteableName() {
 
     @Override
     protected NodeInfo<FieldAttribute> info() {
-        return NodeInfo.create(
-            this,
-            FieldAttribute::new,
-            parentName,
-            name(),
-            dataType(),
-            field,
-            (String) null,
-            nullable(),
-            id(),
-            synthetic()
-        );
+        return NodeInfo.create(this, FieldAttribute::new, parentName, name(), field, nullable(), id(), synthetic());
     }
 
     public String parentName() {

diff --git a/...ql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java b/...ql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java
@@ -147,26 +147,7 @@ protected String label() {
 
     @Override
     protected NodeInfo<? extends Expression> info() {
-        return NodeInfo.create(
-            this,
-            (source, name, dataType, qualifier, nullability, id, synthetic, searchable1) -> new MetadataAttribute(
-                source,
-                name,
-                dataType,
-                qualifier,
-                nullability,
-                id,
-                synthetic,
-                searchable1
-            ),
-            name(),
-            dataType(),
-            (String) null,
-            nullable(),
-            id(),
-            synthetic(),
-            searchable
-        );
+        return NodeInfo.create(this, MetadataAttribute::new, name(), dataType(), nullable(), id(), synthetic(), searchable);
     }
 
     public boolean searchable() {

diff --git a/...l-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java b/...l-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java
@@ -110,24 +110,7 @@ protected Attribute clone(Source source, String name, DataType dataType, Nullabi
 
     @Override
     protected NodeInfo<ReferenceAttribute> info() {
-        return NodeInfo.create(
-            this,
-            (source, name, dataType, qualifier, nullability, id, synthetic) -> new ReferenceAttribute(
-                source,
-                name,
-                dataType,
-                qualifier,
-                nullability,
-                id,
-                synthetic
-            ),
-            name(),
-            dataType(),
-            (String) null,
-            nullable(),
-            id(),
-            synthetic()
-        );
+        return NodeInfo.create(this, ReferenceAttribute::new, name(), dataType(), nullable(), id(), synthetic());
     }
 
     @Override

diff --git a/...ck/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java b/...ck/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java
@@ -21,8 +21,6 @@
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
-import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
-import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttributeTests;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedNamedExpression;
@@ -114,9 +112,13 @@
  * </ul>
  */
 public class EsqlNodeSubclassTests<T extends B, B extends Node<B>> extends NodeSubclassTests {
+    private static final String ESQL_CORE_CLASS_PREFIX = "org.elasticsearch.xpack.esql.core";
+    private static final String ESQL_CORE_JAR_LOCATION_SUBSTRING = "x-pack-esql-core";
+    private static final String ESQL_CLASS_PREFIX = "org.elasticsearch.xpack.esql";
+
     private static final Predicate<String> CLASSNAME_FILTER = className -> {
-        boolean esqlCore = className.startsWith("org.elasticsearch.xpack.esql.core") != false;
-        boolean esqlProper = className.startsWith("org.elasticsearch.xpack.esql") != false;
+        boolean esqlCore = className.startsWith(ESQL_CORE_CLASS_PREFIX) != false;
+        boolean esqlProper = className.startsWith(ESQL_CLASS_PREFIX) != false;
         return (esqlCore || esqlProper);
     };
 
@@ -164,15 +166,6 @@ public void testInfoParameters() throws Exception {
          */
         expectedCount -= 1;
 
-        // special exceptions with private constructors
-        if (MetadataAttribute.class.equals(subclass) || ReferenceAttribute.class.equals(subclass)) {
-            expectedCount++;
-        }
-
-        if (FieldAttribute.class.equals(subclass)) {
-            expectedCount += 2;
-        }
-
         assertEquals(expectedCount, info(node).properties().size());
     }
 
@@ -736,7 +729,7 @@ public static <T> Set<Class<? extends T>> subclassesOf(Class<T> clazz, Predicate
             // NIO FileSystem API is not used since it trips the SecurityManager
             // https://bugs.openjdk.java.net/browse/JDK-8160798
             // so iterate the jar "by hand"
-            if (path.endsWith(".jar") && path.contains("x-pack-ql")) {
+            if (path.endsWith(".jar") && path.contains(ESQL_CORE_JAR_LOCATION_SUBSTRING)) {
                 try (JarInputStream jar = jarStream(root)) {
                     JarEntry je = null;
                     while ((je = jar.getNextJarEntry()) != null) {