EQL: Change query folding spec from new lines to ; (#54882)

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.

(cherry picked from commit 192ad88d3a51e1e1f1f82830526518720ec88217)

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.BufferedReader;
 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 class QueryFolderOkTests extends AbstractQueryFolderTestCase {
 
     @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;
+            ArrayList<Object> expectations = new ArrayList<>(8);
-            int newLineCount = 0;
+
+            StringBuilder sb = new StringBuilder();
 
             while ((line = reader.readLine()) != null) {
-                if (line.startsWith("//")) {
+                lineNumber++;
-                    continue;
-                }
-
                 line = line.trim();
-                if (Strings.isEmpty(line)) {
+
-                    if (name != null) {
+                if (line.isEmpty() || line.startsWith("//")) {
-                        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;
-                    }
                     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
-                if (query == null) {
+                                + " (previously seen at line " + previousName + ")");
-                    query = line;
+                    }
-                    continue;
+                }
-                }
+
-
+                else if (query == null) {
-                if (line.equals("null") == false) {  // special case for no expectations
+                    sb.append(line);
-                    if (expectations == null) {
+                    if (line.endsWith(";")) {
-                        expectations = new ArrayList<>();
+                        sb.setLength(sb.length() - 1);
+                        query = sb.toString();
+                        sb.setLength(0);
+                    }
+                }
+
+                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

@@ -1,122 +1,135 @@
 //
 // QueryFolder test
-// Simple format of the following blocks, separated by two new lines
+//
+// A test is made up of a name (one line), a query that can span multiple lines and ends with ; and one or multiple assertions (one per line) that end with ;
+//
 // <name>
-// <eql query>
+// <eql query>;
 // <expectation 1>
 // <expectation 2>
 // ...
 // <expectation n>
+// ;
 
 
 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*"
-
+;