Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the mismatching default configs in integration tests #11283

Merged
merged 41 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
e3d6bff
Add a new interface to retrieve all configs with their defaults; Add …
jihoonson Jul 29, 2024
e7f1ec5
address comments
jihoonson Aug 1, 2024
37c0f92
missing version update in 2.13 pom
jihoonson Aug 1, 2024
85f05f3
fix match arms
jihoonson Aug 1, 2024
6ba7300
take the json file path as an input
jihoonson Aug 1, 2024
8dbd354
add the new config file in the assembly
jihoonson Aug 1, 2024
cb88561
missing 2.13 change
jihoonson Aug 1, 2024
3a26d15
use maven build directory var
jihoonson Aug 2, 2024
5d59de7
revert unintended change
jihoonson Aug 2, 2024
a7a04d0
remove unnecessary clean
jihoonson Aug 2, 2024
a7b5062
Add a new interface to retrieve all configs with their defaults; Add …
jihoonson Jul 29, 2024
4483745
address comments
jihoonson Aug 1, 2024
52fce4c
missing version update in 2.13 pom
jihoonson Aug 1, 2024
35735c3
fix match arms
jihoonson Aug 1, 2024
9f9ef1e
take the json file path as an input
jihoonson Aug 1, 2024
d73581e
add the new config file in the assembly
jihoonson Aug 1, 2024
99ea474
missing 2.13 change
jihoonson Aug 1, 2024
519bb7d
use maven build directory var
jihoonson Aug 2, 2024
c3ec423
revert unintended change
jihoonson Aug 2, 2024
076351c
remove unnecessary clean
jihoonson Aug 2, 2024
b02faa2
Add things in RapidsConf
jihoonson Aug 2, 2024
4a3c862
missing change for 2.13
jihoonson Aug 5, 2024
8a7f316
fix directory path for scala 2.13
jihoonson Aug 5, 2024
ddb145a
exclude jackson from spark-hive
jihoonson Aug 5, 2024
0542fdf
missing change for 2.13
jihoonson Aug 5, 2024
dab6f76
exclude old jackson stuff from iceberg
jihoonson Aug 6, 2024
49a63d6
copyrights
jihoonson Aug 6, 2024
31306f5
antrun
jihoonson Aug 7, 2024
c02e1c4
fix config file path
jihoonson Aug 7, 2024
229528f
Merge branch 'fix-it-test-defaults-4' into fix-it-test-defaults
jihoonson Aug 7, 2024
b04484a
move most dump changes to rapids conf
gerashegalov Aug 7, 2024
3e59847
clean up after merge
jihoonson Aug 7, 2024
fa380fb
scala 2.13
jihoonson Aug 7, 2024
64b58d9
more strict arg check
jihoonson Aug 7, 2024
f336839
unpack ambiguous string arguments
jihoonson Aug 8, 2024
c4ba1a2
allow legacy negative scale for decimals for some tests
jihoonson Aug 9, 2024
45beca3
should fork for RapidsConf
jihoonson Aug 9, 2024
d45dd1e
remove System.exit() from RapidsConf.main()
jihoonson Aug 9, 2024
951c5f3
missing change for scala 2.13
jihoonson Aug 9, 2024
ab5d8b4
Fix more tests to set configs
jihoonson Aug 12, 2024
04d86b8
add back explicit configs
jihoonson Aug 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion integration_tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2020-2023, NVIDIA CORPORATION.
# Copyright (c) 2020-2024, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -62,3 +62,6 @@ def pytest_addoption(parser):
parser.addoption(
"--pyarrow_test", action='store_true', default=False, help="if enable pyarrow tests"
)
parser.addoption(
"--default_configs_path", action="store", default=None, help="path to a JSON file that stores default configs for integration test"
)
37 changes: 36 additions & 1 deletion integration_tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@
<!-- for hive udf test cases -->
<groupId>org.apache.spark</groupId>
<artifactId>spark-hive_${scala.binary.version}</artifactId>
<exclusions>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
</exclusion>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</exclusion>
</exclusions>
</dependency>
<!-- https://mvnrepository.com/artifact/com.github.scopt/scopt -->
<dependency>
Expand All @@ -78,6 +88,31 @@
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-antrun-plugin</artifactId>
<executions>
<execution>
<id>populate-default-configs-for-testing</id>
<phase>generate-test-resources</phase>
<goals>
<goal>run</goal>
</goals>
<configuration>
<target>
<taskdef resource="net/sf/antcontrib/antcontrib.properties"/>
<java classname="com.nvidia.spark.rapids.tests.DumpDefaultConfigs"
failonerror="true"
classpathref="maven.compile.classpath"
fork="true">
<arg value="json"/>
<arg value="${project.build.directory}/spark-rapids-default-configs.json"/>
</java>
</target>
Comment on lines +102 to +111
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why we end up not needing this #11283 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I'm not quite sure why it works. I can add those java options here just in case..

</configuration>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-assembly-plugin</artifactId>
<version>3.6.0</version>
Expand Down Expand Up @@ -181,7 +216,7 @@
<artifactId>exec-maven-plugin</artifactId>
<executions>
<execution>
<id>run pyspark tests</id>
<id>run-pyspark-tests</id>
<phase>verify</phase><!--run after packageing and collecting dependencies-->
<goals>
<goal>exec</goal>
Expand Down
2 changes: 2 additions & 0 deletions integration_tests/run_pyspark_from_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ else

REPORT_CHARS=${REPORT_CHARS:="fE"} # default as (f)ailed, (E)rror
STD_INPUT_PATH="$INPUT_PATH"/src/test/resources
DEFAULT_CONFIGS_PATH=${DEFAULT_CONFIGS_PATH:-${TARGET_DIR}/spark-rapids-default-configs.json}
TEST_COMMON_OPTS=(-v
-r"$REPORT_CHARS"
"$TEST_TAGS"
Expand All @@ -232,6 +233,7 @@ else
"$TEST_ARGS"
$RUN_TEST_PARAMS
--junitxml=TEST-pytest-`date +%s%N`.xml
--default_configs_path="${DEFAULT_CONFIGS_PATH}"
"$@")

NUM_LOCAL_EXECS=${NUM_LOCAL_EXECS:-0}
Expand Down
6 changes: 5 additions & 1 deletion integration_tests/src/assembly/bin.xml
jihoonson marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright (c) 2020-2023, NVIDIA CORPORATION.
Copyright (c) 2020-2024, NVIDIA CORPORATION.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -50,6 +50,10 @@
<source>${project.build.outputDirectory}/rapids4spark-version-info.properties</source>
<outputDirectory>integration_tests</outputDirectory>
</file>
<file>
<source>${project.build.directory}/spark-rapids-default-configs.json</source>
<outputDirectory>integration_tests</outputDirectory>
jihoonson marked this conversation as resolved.
Show resolved Hide resolved
</file>
</files>
<fileSets>
<fileSet>
Expand Down
Loading