Skip to content

Commit 192ad88

Browse files
costincostin
authored
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.
1 parent c355fea commit 192ad88

File tree

2 files changed

+96
-59
lines changed

2 files changed

+96
-59
lines changed

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

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package org.elasticsearch.xpack.eql.planner;
88

99
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
10+
1011
import org.elasticsearch.common.Strings;
1112
import org.elasticsearch.xpack.eql.plan.physical.EsQueryExec;
1213
import org.elasticsearch.xpack.eql.plan.physical.PhysicalPlan;
@@ -15,6 +16,8 @@
1516
import java.io.InputStreamReader;
1617
import java.nio.charset.StandardCharsets;
1718
import java.util.ArrayList;
19+
import java.util.LinkedHashMap;
20+
import java.util.Map;
1821

1922
import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD;
2023
import static org.hamcrest.Matchers.containsString;
@@ -33,54 +36,73 @@ public QueryFolderOkTests(String name, String query, Object expect) {
3336

3437
@ParametersFactory(shuffle = false, argumentFormatting = "%1$s")
3538
public static Iterable<Object[]> parameters() throws Exception {
39+
return readSpec("/queryfolder_tests.txt");
40+
}
41+
42+
public static Iterable<Object[]> readSpec(String url) throws Exception {
3643
ArrayList<Object[]> arr = new ArrayList<>();
44+
Map<String, Integer> testNames = new LinkedHashMap<>();
45+
3746
try (BufferedReader reader = new BufferedReader(new InputStreamReader(
38-
QueryFolderOkTests.class.getResourceAsStream("/queryfolder_tests.txt"), StandardCharsets.UTF_8))) {
47+
QueryFolderOkTests.class.getResourceAsStream(url), StandardCharsets.UTF_8))) {
48+
int lineNumber = 0;
3949
String line;
4050
String name = null;
4151
String query = null;
42-
ArrayList<Object> expectations = null;
43-
int newLineCount = 0;
52+
ArrayList<Object> expectations = new ArrayList<>(8);
4453

45-
while ((line = reader.readLine()) != null) {
46-
if (line.startsWith("//")) {
47-
continue;
48-
}
54+
StringBuilder sb = new StringBuilder();
4955

56+
while ((line = reader.readLine()) != null) {
57+
lineNumber++;
5058
line = line.trim();
51-
if (Strings.isEmpty(line)) {
52-
if (name != null) {
53-
newLineCount++;
54-
}
55-
if (newLineCount >= 2) {
56-
// Add and zero out for the next spec
57-
addSpec(arr, name, query, expectations == null ? null : expectations.toArray());
58-
name = null;
59-
query = null;
60-
expectations = null;
61-
newLineCount = 0;
62-
}
59+
60+
if (line.isEmpty() || line.startsWith("//")) {
6361
continue;
6462
}
6563

6664
if (name == null) {
6765
name = line;
68-
continue;
66+
Integer previousName = testNames.put(name, lineNumber);
67+
if (previousName != null) {
68+
throw new IllegalArgumentException("Duplicate test name '" + line + "' at line " + lineNumber
69+
+ " (previously seen at line " + previousName + ")");
70+
}
6971
}
7072

71-
if (query == null) {
72-
query = line;
73-
continue;
73+
else if (query == null) {
74+
sb.append(line);
75+
if (line.endsWith(";")) {
76+
sb.setLength(sb.length() - 1);
77+
query = sb.toString();
78+
sb.setLength(0);
79+
}
7480
}
7581

76-
if (line.equals("null") == false) { // special case for no expectations
77-
if (expectations == null) {
78-
expectations = new ArrayList<>();
82+
else {
83+
boolean done = false;
84+
if (line.endsWith(";")) {
85+
line = line.substring(0, line.length() - 1);
86+
done = true;
87+
}
88+
// no expectation
89+
if (line.equals("null") == false) {
90+
expectations.add(line);
91+
}
92+
93+
if (done) {
94+
// Add and zero out for the next spec
95+
addSpec(arr, name, query, expectations.isEmpty() ? null : expectations.toArray());
96+
name = null;
97+
query = null;
98+
expectations.clear();
7999
}
80-
expectations.add(line);
81100
}
82101
}
83-
addSpec(arr, name, query, expectations.toArray());
102+
103+
if (name != null) {
104+
throw new IllegalStateException("Read a test [" + name + "] without a body at the end of [" + url + "]");
105+
}
84106
}
85107
return arr;
86108
}

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

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,128 +1,143 @@
11
//
22
// QueryFolder test
3-
// Simple format of the following blocks, separated by two new lines
3+
//
4+
// 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 ;
5+
//
46
// <name>
5-
// <eql query>
7+
// <eql query>;
68
// <expectation 1>
79
// <expectation 2>
810
// ...
911
// <expectation n>
12+
// ;
1013

1114

1215
basic
13-
process where true
16+
process where true;
1417
null
15-
18+
;
1619

1720
singleNumericFilterEquals
18-
process where serial_event_id = 1
21+
process where serial_event_id = 1;
1922
"term":{"serial_event_id":{"value":1
20-
23+
;
2124

2225
singleNumericFilterLess
23-
process where serial_event_id < 4
26+
process where serial_event_id < 4;
2427
"range":{"serial_event_id":{"from":null,"to":4,"include_lower":false,"include_upper":false
25-
28+
;
2629

2730
singleNumericFilterLessEquals
28-
process where serial_event_id <= 4
31+
process where serial_event_id <= 4;
2932
"range":{"serial_event_id":{"from":null,"to":4,"include_lower":false,"include_upper":true
30-
33+
;
3134

3235
singleNumericFilterGreater
33-
process where serial_event_id > 4
36+
process where serial_event_id > 4;
3437
"range":{"serial_event_id":{"from":4,"to":null,"include_lower":false,"include_upper":false
35-
38+
;
3639

3740
singleNumericFilterGreaterEquals
38-
process where serial_event_id >= 4
41+
process where serial_event_id >= 4;
3942
"range":{"serial_event_id":{"from":4,"to":null,"include_lower":true,"include_upper":false
40-
43+
;
4144

4245
mixedTypeFilter
43-
process where process_name == "notepad.exe" or (serial_event_id < 4.5 and serial_event_id >= 3.1)
46+
process where process_name == "notepad.exe" or (serial_event_id < 4.5 and serial_event_id >= 3.1);
4447
"term":{"process_name":{"value":"notepad.exe"
4548
"range":{"serial_event_id":{"from":3.1,"to":4.5,"include_lower":true,"include_upper":false
46-
49+
;
4750

4851
notFilter
49-
process where not (exit_code > -1)
52+
process where not (exit_code > -1);
5053
"range":{"exit_code":{"from":null,"to":-1,"include_lower":false,"include_upper":true
51-
54+
;
5255

5356
inFilter
54-
process where process_name in ("python.exe", "SMSS.exe", "explorer.exe")
57+
process where process_name in ("python.exe", "SMSS.exe", "explorer.exe");
5558
"terms":{"process_name":["python.exe","SMSS.exe","explorer.exe"],
56-
59+
;
5760

5861
equalsAndInFilter
5962
process where process_path == "*\\red_ttp\\wininit.*" and opcode in (0,1,2,3)
63+
;
6064
"wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
6165
{"terms":{"opcode":[0,1,2,3]
62-
66+
;
6367

6468
endsWithFunction
6569
process where endsWith(user_name, 'c')
70+
;
6671
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.endsWith(
6772
InternalQlScriptUtils.docValue(doc,params.v0),params.v1))",
6873
"params":{"v0":"user_name","v1":"c"}
69-
74+
;
7075

7176
lengthFunctionWithExactSubField
7277
process where length(file_name) > 0
78+
;
7379
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.gt(
7480
InternalEqlScriptUtils.length(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))",
7581
"params":{"v0":"file_name.keyword","v1":0}
76-
82+
;
7783

7884
lengthFunctionWithExactField
7985
process where 12 == length(user_name)
86+
;
8087
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.eq(
8188
InternalEqlScriptUtils.length(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))",
8289
"params":{"v0":"user_name","v1":12}
83-
90+
;
8491

8592
lengthFunctionWithConstantKeyword
8693
process where 5 > length(constant_keyword)
94+
;
8795
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.lt(
8896
InternalEqlScriptUtils.length(InternalQlScriptUtils.docValue(doc,params.v0)),params.v1))",
8997
"params":{"v0":"constant_keyword","v1":5}
90-
98+
;
9199

92100
startsWithFunction
93101
process where startsWith(user_name, 'A')
102+
;
94103
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.startsWith(
95104
InternalQlScriptUtils.docValue(doc,params.v0),params.v1))",
96105
"params":{"v0":"user_name","v1":"A"}
97-
106+
;
98107

99108
substringFunction
100109
process where substring(file_name, -4) == '.exe'
110+
;
101111
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.eq(
102112
InternalEqlScriptUtils.substring(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2),params.v3))",
103113
"params":{"v0":"file_name.keyword","v1":-4,"v2":null,"v3":".exe"}
104-
114+
;
105115

106116
betweenFunction
107-
process where between(process_name, "s", "e") == "yst"
117+
process where between(process_name, "s", "e") == "yst";
118+
108119
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.eq(
109120
InternalEqlScriptUtils.between(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2,params.v3,params.v4),params.v5))",
110121
"params":{"v0":"process_name","v1":"s","v2":"e","v3":false,"v4":false,"v5":"yst"}
111-
122+
;
112123

113124
wildcardFunctionSingleArgument
114125
process where wildcard(process_path, "*\\red_ttp\\wininit.*")
126+
;
115127
"wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
116-
128+
;
117129

118130
wildcardFunctionTwoArguments
119131
process where wildcard(process_path, "*\\red_ttp\\wininit.*", "*\\abc\\*")
132+
;
120133
"wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
121134
"wildcard":{"process_path":{"wildcard":"*\\\\abc\\\\*"
122-
135+
;
123136

124137
wildcardFunctionThreeArguments
125138
process where wildcard(process_path, "*\\red_ttp\\wininit.*", "*\\abc\\*", "*def*")
139+
;
126140
"wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
127141
"wildcard":{"process_path":{"wildcard":"*\\\\abc\\\\*"
128142
"wildcard":{"process_path":{"wildcard":"*def*"
143+
;

0 commit comments

Comments
 (0)