EQL: Change query folding spec from new lines to ;

The usage of blank lines as separator between tests can be tricky to
deal with in case of merges where such lines can be added by accident.
Further more counting non-consecutive lines is non-intuitive.
The tests have been aligned to use ; at the end of the query and
exceptions so that the presence or absence of empty lines is irrelevant.
The parsing of the spec has been changed to perform validation to not
allow invalid/incomplete specs to cause exceptions.

Author: costin

x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderOkTests.java

@@ -7,6 +7,7 @@
 package org.elasticsearch.xpack.eql.planner;
 
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
+
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.xpack.eql.plan.physical.EsQueryExec;
 import org.elasticsearch.xpack.eql.plan.physical.PhysicalPlan;
@@ -15,6 +16,8 @@
 import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.Map;
 
 import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;
 import static org.hamcrest.Matchers.containsString;
@@ -33,54 +36,73 @@ public QueryFolderOkTests(String name, String query, Object expect) {
 
     @ParametersFactory(shuffle = false, argumentFormatting = "%1$s")
     public static Iterable<Object[]> parameters() throws Exception {
+        return readSpec("/queryfolder_tests.txt");
+    }
+
+    public static Iterable<Object[]> readSpec(String url) throws Exception {
         ArrayList<Object[]> arr = new ArrayList<>();
+        Map<String, Integer> testNames = new LinkedHashMap<>();
+
         try (BufferedReader reader = new BufferedReader(new InputStreamReader(
-                QueryFolderOkTests.class.getResourceAsStream("/queryfolder_tests.txt"), StandardCharsets.UTF_8))) {
+                QueryFolderOkTests.class.getResourceAsStream(url), StandardCharsets.UTF_8))) {
+            int lineNumber = 0;
             String line;
             String name = null;
             String query = null;
-            ArrayList<Object> expectations = null;
-            int newLineCount = 0;
+            ArrayList<Object> expectations = new ArrayList<>(8);
 
-            while ((line = reader.readLine()) != null) {
-                if (line.startsWith("//")) {
-                    continue;
-                }
+            StringBuilder sb = new StringBuilder();
 
+            while ((line = reader.readLine()) != null) {
+                lineNumber++;
                 line = line.trim();
-                if (Strings.isEmpty(line)) {
-                    if (name != null) {
-                        newLineCount++;
-                    }
-                    if (newLineCount >= 2) {
-                        // Add and zero out for the next spec
-                        addSpec(arr, name, query, expectations == null ? null : expectations.toArray());
-                        name = null;
-                        query = null;
-                        expectations = null;
-                        newLineCount = 0;
-                    }
+
+                if (line.isEmpty() || line.startsWith("//")) {
                     continue;
                 }
 
                 if (name == null) {
                     name = line;
-                    continue;
+                    Integer previousName = testNames.put(name, lineNumber);
+                    if (previousName != null) {
+                        throw new IllegalArgumentException("Duplicate test name '" + line + "' at line " + lineNumber
+                                + " (previously seen at line " + previousName + ")");
+                    }
                 }
 
-                if (query == null) {
-                    query = line;
-                    continue;
+                else if (query == null) {
+                    sb.append(line);
+                    if (line.endsWith(";")) {
+                        sb.setLength(sb.length() - 1);
+                        query = sb.toString();
+                        sb.setLength(0);
+                    }
                 }
 
-                if (line.equals("null") == false) {  // special case for no expectations
-                    if (expectations == null) {
-                        expectations = new ArrayList<>();
+                else {
+                    boolean done = false;
+                    if (line.endsWith(";")) {
+                        line = line.substring(0, line.length() - 1);
+                        done = true;
+                    }
+                    // no expectation
+                    if (line.equals("null") == false) {
+                        expectations.add(line);
+                    }
+
+                    if (done) {
+                        // Add and zero out for the next spec
+                        addSpec(arr, name, query, expectations.isEmpty() ? null : expectations.toArray());
+                        name = null;
+                        query = null;
+                        expectations.clear();
                     }
-                    expectations.add(line);
                 }
             }
-            addSpec(arr, name, query, expectations.toArray());
+
+            if (name != null) {
+                throw new IllegalStateException("Read a test [" + name + "] without a body at the end of [" + url + "]");
+            }
         }
         return arr;
     }

x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt

@@ -10,113 +10,123 @@
 
 
 basic
-process where true
+process where true;
 null
-
+;
 
 singleNumericFilterEquals
-process where serial_event_id = 1
+process where serial_event_id = 1;
 "term":{"serial_event_id":{"value":1
-
+;
 
 singleNumericFilterLess
-process where serial_event_id < 4
+process where serial_event_id < 4;
 "range":{"serial_event_id":{"from":null,"to":4,"include_lower":false,"include_upper":false
-
+;
 
 singleNumericFilterLessEquals
-process where serial_event_id <= 4
+process where serial_event_id <= 4;
 "range":{"serial_event_id":{"from":null,"to":4,"include_lower":false,"include_upper":true
-
+;
 
 singleNumericFilterGreater
-process where serial_event_id > 4
+process where serial_event_id > 4;
 "range":{"serial_event_id":{"from":4,"to":null,"include_lower":false,"include_upper":false
-
+;
 
 singleNumericFilterGreaterEquals
-process where serial_event_id >= 4
+process where serial_event_id >= 4;
 "range":{"serial_event_id":{"from":4,"to":null,"include_lower":true,"include_upper":false
-
+;
 
 mixedTypeFilter
-process where process_name == "notepad.exe" or (serial_event_id < 4.5 and serial_event_id >= 3.1)
+process where process_name == "notepad.exe" or (serial_event_id < 4.5 and serial_event_id >= 3.1);
 "term":{"process_name":{"value":"notepad.exe"
 "range":{"serial_event_id":{"from":3.1,"to":4.5,"include_lower":true,"include_upper":false
-
+;
 
 notFilter
-process where not (exit_code > -1)
+process where not (exit_code > -1);
 "range":{"exit_code":{"from":null,"to":-1,"include_lower":false,"include_upper":true
-
+;
 
 inFilter
-process where process_name in ("python.exe", "SMSS.exe", "explorer.exe")
+process where process_name in ("python.exe", "SMSS.exe", "explorer.exe");
 "terms":{"process_name":["python.exe","SMSS.exe","explorer.exe"],
-
+;
 
 equalsAndInFilter
 process where process_path == "*\\red_ttp\\wininit.*" and opcode in (0,1,2,3)
+;
 "wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
 {"terms":{"opcode":[0,1,2,3]
-
+;
 
 endsWithFunction
 process where endsWith(user_name, 'c')
+;
 "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.endsWith(
 InternalQlScriptUtils.docValue(doc,params.v0),params.v1))",
 "params":{"v0":"user_name","v1":"c"}
-
+;
 
 lengthFunctionWithExactSubField
 process where length(file_name) > 0
+;
 "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.gt(
 InternalEqlScriptUtils.length(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))",
 "params":{"v0":"file_name.keyword","v1":0}
-
+;
 
 lengthFunctionWithExactField
 process where 12 == length(user_name)
+;
 "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.eq(
 InternalEqlScriptUtils.length(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))",
 "params":{"v0":"user_name","v1":12}
-
+;
 
 lengthFunctionWithConstantKeyword
 process where 5 > length(constant_keyword)
+;
 "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.lt(
 InternalEqlScriptUtils.length(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))",
 "params":{"v0":"constant_keyword","v1":5}
-
+;
 
 startsWithFunction
 process where startsWith(user_name, 'A')
+;
 "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.startsWith(
 InternalQlScriptUtils.docValue(doc,params.v0),params.v1))",
 "params":{"v0":"user_name","v1":"A"}
-
+;
 
 substringFunction
 process where substring(file_name, -4) == '.exe'
+;
 "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.eq(
 InternalEqlScriptUtils.substring(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2),params.v3))",
 "params":{"v0":"file_name.keyword","v1":-4,"v2":null,"v3":".exe"}
-
+;
 
 wildcardFunctionSingleArgument
 process where wildcard(process_path, "*\\red_ttp\\wininit.*")
+;
 "wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
-
+;
 
 wildcardFunctionTwoArguments
 process where wildcard(process_path, "*\\red_ttp\\wininit.*", "*\\abc\\*")
+;
 "wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
 "wildcard":{"process_path":{"wildcard":"*\\\\abc\\\\*"
-
+;
 
 wildcardFunctionThreeArguments
 process where wildcard(process_path, "*\\red_ttp\\wininit.*", "*\\abc\\*", "*def*")
+;
 "wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
 "wildcard":{"process_path":{"wildcard":"*\\\\abc\\\\*"
 "wildcard":{"process_path":{"wildcard":"*def*"
-
+;