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.

Sign up for GitHub

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

Jump to bottom

initial PR for the framework reusing Vanilla Spark's unit tests #10743

Merged
merged 20 commits into from
May 10, 2024
Merged

initial PR for the framework reusing Vanilla Spark's unit tests #10743

merged 20 commits into from
May 10, 2024

Conversation

binmahone
Copy link
Collaborator

@binmahone binmahone commented Apr 25, 2024 •
edited by jlowe
Loading

(borrowing from Gluten)

Closes #10745

This PR adds a framework to enabling Vanilla Spark UTs by:

  1. Some utility Traits, most importantly RapidsTestsTrait and RapidsSQLTestsTrait for rapids test suite to extend, these traits: injects rapids backboned spark session to the base class, as well as allow testcase level exclusion/inclusion.
  2. A centralized control center called RapidsTestSettings, which decides what suites/testcases are included/excluded.
  3. A first batch of 8 suites are enabled, listed at https://github.com/NVIDIA/spark-rapids/pull/10743/files#diff-4be8dbebdd70379fcc56bae360c6997a9a2b6e66ce48b229c7ff289ae02a5222R41

Most of codes are copied from Apache Gluten, which also follows Apache license.
To respect their work I kept their origin Apache License header in the source code file.

Currently we only focus on spark 3.3.0.

Check out more details at our internal design doc.

@binmahone binmahone changed the title initial PR for the framework resuing Vanila Spark's UTs (borrow from … [WIP] initial PR for the framework resuing Vanila Spark's UTs (borrow from … Apr 25, 2024
@binmahone binmahone marked this pull request as draft April 25, 2024 13:05
@binmahone
fix shim complain
9c1c43f 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
fix other shim compile issue
db56110 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
fix
8740620 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
fix for scala 2.13
e32bcb2 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

@binmahone
more to shim
b33e06d 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
clean pom
fb057f7 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
move everything to shim
e127e39 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
2.13
8d37886 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

@binmahone
move to spark330
42489c2 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
fix
58c4a89 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

@binmahone
fix bug
9f5fc01 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

1 similar comment
@binmahone
Copy link
Collaborator Author

build

@binmahone
fix blossom ci
64d783c 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

@binmahone binmahone marked this pull request as ready for review May 1, 2024 08:26
@binmahone
Copy link
Collaborator Author

Hi @GaryShen2008 @jlowe the PR has passed all CI checks, can you please help to review?

@binmahone binmahone changed the title [WIP] initial PR for the framework resuing Vanila Spark's UTs (borrow from … initial PR for the framework resuing Vanila Spark's UTs (borrow from … May 1, 2024
@jlowe jlowe changed the title initial PR for the framework resuing Vanila Spark's UTs (borrow from … initial PR for the framework reusing Vanilla Spark's unit tests May 1, 2024
jlowe
jlowe reviewed May 1, 2024
View reviewed changes
tests/src/test/java/com/nvidia/spark/rapids/TestStats.java Outdated Show resolved Hide resolved
tests/src/test/java/com/nvidia/spark/rapids/TestStats.java Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/RapidsSQLTestsBaseTrait.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/RapidsSQLTestsBaseTrait.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/RapidsSQLTestsTrait.scala Outdated Show resolved Hide resolved
...c/test/spark330/scala/org/apache/spark/sql/rapids/suites/RapidsDataFrameAggregateSuite.scala Outdated Show resolved Hide resolved
.../src/test/spark330/scala/org/apache/spark/sql/rapids/suites/RapidsStringFunctionsSuite.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/RapidsTestsBaseTrait.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/rapids/utils/BackendTestSettings.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/rapids/utils/SQLQueryTestSettings.scala Outdated Show resolved Hide resolved
@sameerz sameerz added the test Only impacts tests label May 2, 2024
binmahone added 5 commits May 6, 2024 15:54
@binmahone
address license comment
49f9fad 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
address suites util package
c7a3171 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
address reason comment
0d99b76 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
address other comments
69932d8 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
fix bug
f0f1ac4 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

1 similar comment
@binmahone
Copy link
Collaborator Author

build

This was referenced May 7, 2024
Open
Closed
Open
Closed
Closed
jlowe
jlowe reviewed May 7, 2024
View reviewed changes
tests/src/test/spark330/scala/org/apache/spark/sql/rapids/utils/RapidsTestsTrait.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/rapids/utils/RapidsTestSettings.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/RapidsTestSettings.scala Outdated
class RapidsTestSettings extends BackendTestSettings {

enableSuite[RapidsCastSuite]
.exclude("Process Infinity, -Infinity, NaN in case insensitive manner")
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but many things are passing UNKNOWN_ISSUE(), which is essentially not passing a reason. 😸
Is there a followup issue to investigate these? Probably should be at least one, or a series of issues maybe grouped by test type, and then that issue can be linked here and other places UNKNOWN_ISSUE was used.

tests/src/test/spark330/scala/org/apache/spark/sql/rapids/utils/BackendTestSettings.scala Outdated Show resolved Hide resolved
tests/src/test/spark330/scala/org/apache/spark/sql/RapidsTestsTrait.scala Outdated Show resolved Hide resolved
@binmahone
address 2rd round of comments
4bbe74f 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

1 similar comment
@binmahone
Copy link
Collaborator Author

build

@binmahone
get rid of UNKNOW_ISSUE
50b9904 
Signed-off-by: Hongbin Ma (Mahone) <[email protected]>
@binmahone
Copy link
Collaborator Author

build

@binmahone binmahone merged commit 4dee2f8 into NVIDIA:branch-24.06 May 10, 2024
44 checks passed
binmahone added a commit to binmahone/spark-rapids that referenced this pull request May 10, 2024
@binmahone
Revert "initial PR for the framework reusing Vanilla Spark's unit tes…
8540729 
…ts (NVIDIA#10743)"

This reverts commit 4dee2f8.
@binmahone binmahone mentioned this pull request May 10, 2024
Closed
@binmahone binmahone mentioned this pull request May 24, 2024
Closed
8 tasks
@GaryShen2008 GaryShen2008 mentioned this pull request Jul 5, 2024
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe approved these changes

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

@tgravescs tgravescs Awaiting requested review from tgravescs tgravescs is a code owner

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

@gerashegalov gerashegalov Awaiting requested review from gerashegalov gerashegalov is a code owner

Assignees
No one assigned
Labels
test Only impacts tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Initialize a test framework to run Spark UT in Spark 3.3.0 shims
3 participants
@binmahone @jlowe @sameerz