Skip to content

Commit f614cb6

Browse files
pramodsatyaManoj Negi
pramodsatya
and
Manoj Negi
committed
Add presto-native-tests module
Co-authored-by: Manoj Negi <[email protected]>
1 parent 152e64d commit f614cb6

21 files changed

+3516
-4
lines changed

‎.circleci/continue_config.yml‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ workflows:
2929
run_linux_tests: << pipeline.parameters.run_linux_tests >>
3030
requires:
3131
- linux-build-and-unit-test
32+
- linux-presto-native-tests:
33+
run_linux_tests: << pipeline.parameters.run_linux_tests >>
34+
requires:
35+
- linux-build-and-unit-test
3236
- linux-spark-e2e-tests:
3337
run_linux_tests: << pipeline.parameters.run_linux_tests >>
3438
requires:
@@ -193,6 +197,54 @@ jobs:
193197
- store_artifacts:
194198
path: '/tmp/PrestoNativeQueryRunnerUtils'
195199

200+
linux-presto-native-tests:
201+
executor: build
202+
parameters:
203+
run_linux_tests:
204+
type: boolean
205+
default: false
206+
parallelism: 5
207+
steps:
208+
- run: echo "Run Linux tests is << parameters.run_linux_tests >>"
209+
- when:
210+
condition: << parameters.run_linux_tests >>
211+
steps:
212+
- checkout
213+
- attach_workspace:
214+
at: presto-native-tests
215+
- maven_install:
216+
maven_install_opts: ${MAVEN_INSTALL_OPTS}
217+
maven_fast_install: ${MAVEN_FAST_INSTALL}
218+
- run:
219+
name: 'Run Presto native tests'
220+
command: |
221+
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/usr/local/lib:/usr/local/lib64"
222+
export PRESTO_SERVER_PATH="${HOME}/project/presto-native-execution/_build/debug/presto_cpp/main/presto_server"
223+
export TEMP_PATH="/tmp"
224+
TESTFILES=$(circleci tests glob "presto-native-tests/src/test/**/Test*.java" | circleci tests split --split-by=timings)
225+
# Convert file paths to comma separated class names
226+
export TESTCLASSES=
227+
for test_file in $TESTFILES
228+
do
229+
tmp=${test_file##*/}
230+
test_class=${tmp%%\.*}
231+
export TESTCLASSES="${TESTCLASSES},$test_class"
232+
done
233+
export TESTCLASSES=${TESTCLASSES#,}
234+
235+
if [ ! -z $TESTCLASSES ]; then
236+
mvn test \
237+
${MAVEN_TEST} \
238+
-pl 'presto-native-tests' \
239+
-Dtest="${TESTCLASSES}" \
240+
-DPRESTO_SERVER=${PRESTO_SERVER_PATH} \
241+
-DDATA_DIR=${TEMP_PATH} \
242+
-Duser.timezone=America/Bahia_Banderas \
243+
-T1C
244+
fi
245+
- store_artifacts:
246+
path: '/tmp/PrestoNativeQueryRunnerUtils'
247+
196248
linux-spark-e2e-tests:
197249
executor: build
198250
parameters:

‎.github/workflows/test-other-modules.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ jobs:
6464
run: |
6565
./mvnw test -T 1 ${MAVEN_TEST} -pl '
6666
!presto-tests,
67+
!presto-native-tests,
6768
!presto-accumulo,
6869
!presto-cassandra,
6970
!presto-hive,

‎pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@
193193
<module>presto-test-coverage</module>
194194
<module>presto-hudi</module>
195195
<module>presto-native-execution</module>
196+
<module>presto-native-tests</module>
196197
<module>presto-router</module>
197198
<module>presto-open-telemetry</module>
198199
<module>redis-hbo-provider</module>

‎presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import java.nio.file.Files;
3131
import java.nio.file.Path;
3232
import java.nio.file.Paths;
33+
import java.util.Collections;
34+
import java.util.Map;
3335
import java.util.Optional;
3436
import java.util.OptionalInt;
3537
import java.util.UUID;
@@ -88,7 +90,7 @@ public static QueryRunner createQueryRunner(
8890

8991
defaultQueryRunner.close();
9092

91-
return createNativeQueryRunner(dataDirectory.get().toString(), prestoServerPath.get(), workerCount, cacheMaxSize, true, Optional.empty(), storageFormat, addStorageFormatToPath, false);
93+
return createNativeQueryRunner(dataDirectory.get().toString(), prestoServerPath.get(), workerCount, cacheMaxSize, true, Optional.empty(), storageFormat, addStorageFormatToPath, false, Collections.emptyMap(), Collections.emptyMap());
9294
}
9395

9496
public static QueryRunner createJavaQueryRunner()
@@ -265,7 +267,9 @@ public static QueryRunner createNativeQueryRunner(
265267
Optional<String> remoteFunctionServerUds,
266268
String storageFormat,
267269
boolean addStorageFormatToPath,
268-
Boolean failOnNestedLoopJoin)
270+
Boolean failOnNestedLoopJoin,
271+
Map<String, String> extraProperties,
272+
Map<String, String> extraCoordinatorProperties)
269273
throws Exception
270274
{
271275
// The property "hive.allow-drop-table" needs to be set to true because security is always "legacy" in NativeQueryRunner.
@@ -282,8 +286,11 @@ public static QueryRunner createNativeQueryRunner(
282286
.put("http-server.http.port", "8081")
283287
.put("experimental.internal-communication.thrift-transport-enabled", String.valueOf(useThrift))
284288
.putAll(getNativeWorkerSystemProperties())
289+
.putAll(extraProperties)
290+
.build(),
291+
ImmutableMap.<String, String>builder()
292+
.putAll(extraCoordinatorProperties)
285293
.build(),
286-
ImmutableMap.of(),
287294
"legacy",
288295
hiveProperties,
289296
workerCount,
@@ -340,6 +347,26 @@ public static QueryRunner createNativeQueryRunner(String remoteFunctionServerUds
340347
return createNativeQueryRunner(false, DEFAULT_STORAGE_FORMAT, Optional.ofNullable(remoteFunctionServerUds), false);
341348
}
342349

350+
public static QueryRunner createNativeQueryRunner(Map<String, String> extraProperties, Map<String, String> extraCoordinatorProperties,
351+
String storageFormat)
352+
throws Exception
353+
{
354+
int cacheMaxSize = 0;
355+
NativeQueryRunnerParameters nativeQueryRunnerParameters = getNativeQueryRunnerParameters();
356+
return createNativeQueryRunner(
357+
nativeQueryRunnerParameters.dataDirectory.toString(),
358+
nativeQueryRunnerParameters.serverBinary.toString(),
359+
nativeQueryRunnerParameters.workerCount,
360+
cacheMaxSize,
361+
true,
362+
Optional.empty(),
363+
storageFormat,
364+
true,
365+
false,
366+
extraProperties,
367+
extraCoordinatorProperties);
368+
}
369+
343370
public static QueryRunner createNativeQueryRunner(boolean useThrift)
344371
throws Exception
345372
{
@@ -372,7 +399,9 @@ public static QueryRunner createNativeQueryRunner(boolean useThrift, String stor
372399
remoteFunctionServerUds,
373400
storageFormat,
374401
true,
375-
failOnNestedLoopJoin);
402+
failOnNestedLoopJoin,
403+
Collections.emptyMap(),
404+
Collections.emptyMap());
376405
}
377406

378407
// Start the remote function server. Return the UDS path used to communicate with it.

‎presto-native-tests/README.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Presto Native Tests
2+
3+
This module contains end-to-end tests that run queries from test classes in
4+
the `presto-tests` module with Presto C++ workers. Please build the module
5+
`presto-native-execution` first.
6+
7+
The following command can be used to run all tests in this module:
8+
```
9+
mvn test
10+
-pl 'presto-native-tests'
11+
-Dtest="com.facebook.presto.nativetests.Test*"
12+
-Duser.timezone=America/Bahia_Banderas
13+
-DPRESTO_SERVER=${PRESTO_HOME}/presto-native-execution/cmake-build-debug/presto_cpp/main/presto_server
14+
-DWORKER_COUNT=${WORKER_COUNT} -T1C
15+
```
16+
Please update JVM argument `PRESTO_SERVER` to point to the Presto C++ worker
17+
binary `presto_server`.
18+
19+
## Adding new tests
20+
21+
Presto C++ currently does not have the same behavior as Presto for certain
22+
queries. This could be because of missing types, missing function signatures,
23+
among other reasons. Tests with these unsupported queries are therefore
24+
expected to fail and the test asserts the error message is as expected.
25+
26+
Issues should also be created for the failing queries, so they are documented
27+
and fixed. Please add the tag `presto-native-tests` for these issues.
28+
Once all the failures in a testcase are fixed, the overriden test in this
29+
module should be removed and the testcase in the corresponding base class in
30+
`presto-tests` would be the single source of truth for Presto SQL coverage
31+
tests.

‎presto-native-tests/pom.xml‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
5+
<parent>
6+
<groupId>com.facebook.presto</groupId>
7+
<artifactId>presto-root</artifactId>
8+
<version>0.290-SNAPSHOT</version>
9+
</parent>
10+
11+
<artifactId>presto-native-tests</artifactId>
12+
<name>presto-native-tests</name>
13+
<description>Presto Native Tests</description>
14+
15+
<properties>
16+
<air.main.basedir>${project.parent.basedir}</air.main.basedir>
17+
</properties>
18+
19+
<dependencies>
20+
<dependency>
21+
<groupId>org.testng</groupId>
22+
<artifactId>testng</artifactId>
23+
</dependency>
24+
25+
<dependency>
26+
<groupId>com.facebook.presto</groupId>
27+
<artifactId>presto-native-execution</artifactId>
28+
<version>0.290-SNAPSHOT</version>
29+
<type>test-jar</type>
30+
<scope>test</scope>
31+
</dependency>
32+
33+
<dependency>
34+
<groupId>com.facebook.presto</groupId>
35+
<artifactId>presto-common</artifactId>
36+
</dependency>
37+
38+
<!-- Presto SPI -->
39+
<dependency>
40+
<groupId>com.facebook.presto</groupId>
41+
<artifactId>presto-spi</artifactId>
42+
</dependency>
43+
44+
<dependency>
45+
<groupId>com.facebook.presto</groupId>
46+
<artifactId>presto-main</artifactId>
47+
</dependency>
48+
49+
<dependency>
50+
<groupId>com.facebook.presto</groupId>
51+
<artifactId>presto-tests</artifactId>
52+
</dependency>
53+
54+
<dependency>
55+
<groupId>com.google.guava</groupId>
56+
<artifactId>guava</artifactId>
57+
</dependency>
58+
59+
<dependency>
60+
<groupId>com.facebook.presto</groupId>
61+
<artifactId>presto-tpcds</artifactId>
62+
<scope>test</scope>
63+
</dependency>
64+
65+
<dependency>
66+
<groupId>org.jetbrains</groupId>
67+
<artifactId>annotations</artifactId>
68+
<scope>test</scope>
69+
</dependency>
70+
</dependencies>
71+
72+
<build>
73+
<plugins>
74+
<!-- Disable git-commit-id-plugin plugin to allow for running tests without
75+
a git checkout -->
76+
<plugin>
77+
<groupId>pl.project13.maven</groupId>
78+
<artifactId>git-commit-id-plugin</artifactId>
79+
<configuration>
80+
<skip>true</skip>
81+
</configuration>
82+
</plugin>
83+
<plugin>
84+
<groupId>org.basepom.maven</groupId>
85+
<artifactId>duplicate-finder-maven-plugin</artifactId>
86+
<configuration>
87+
<ignoredResourcePatterns>
88+
<ignoredResourcePattern>parquet.thrift</ignoredResourcePattern>
89+
<ignoredResourcePattern>about.html</ignoredResourcePattern>
90+
<ignoredResourcePattern>mozilla/public-suffix-list.txt</ignoredResourcePattern>
91+
<ignoredResourcePattern>iceberg-build.properties</ignoredResourcePattern>
92+
<ignoredResourcePattern>org.apache.avro.data/Json.avsc</ignoredResourcePattern>
93+
</ignoredResourcePatterns>
94+
<ignoredClassPatterns>
95+
<ignoredClassPattern>com.esotericsoftware.kryo.*</ignoredClassPattern>
96+
<ignoredClassPattern>com.esotericsoftware.minlog.Log</ignoredClassPattern>
97+
<ignoredClassPattern>com.esotericsoftware.reflectasm.*</ignoredClassPattern>
98+
<ignoredClassPattern>module-info</ignoredClassPattern>
99+
<ignoredClassPattern>META-INF.versions.9.module-info</ignoredClassPattern>
100+
<ignoredClassPattern>org.apache.avro.*</ignoredClassPattern>
101+
<ignoredClassPattern>com.github.benmanes.caffeine.*</ignoredClassPattern>
102+
<ignoredClassPattern>org.roaringbitmap.*</ignoredClassPattern>
103+
</ignoredClassPatterns>
104+
</configuration>
105+
</plugin>
106+
<plugin>
107+
<groupId>org.apache.maven.plugins</groupId>
108+
<artifactId>maven-surefire-plugin</artifactId>
109+
<configuration>
110+
<argLine>-Xms4g -Xmx4g</argLine>
111+
<forkCount>1</forkCount>
112+
<reuseForks>false</reuseForks>
113+
<excludedGroups>remote-function,textfile_reader</excludedGroups>
114+
<systemPropertyVariables>
115+
<PRESTO_SERVER>/root/project/build/debug/presto_cpp/main/presto_server</PRESTO_SERVER>
116+
</systemPropertyVariables>
117+
</configuration>
118+
</plugin>
119+
</plugins>
120+
</build>
121+
</project>

0 commit comments

Comments
 (0)