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 test failure with Spark 3.3 by looking for less specific error message #5185

Merged
merged 2 commits into from
Apr 11, 2022

Conversation

andygrove
Copy link
Contributor

@andygrove andygrove commented Apr 8, 2022

Closes #5182

Spark 3.3 changed which exception they throw for a divide that causes an overflow in apache/spark@c2ed15d, causing our tests to fail.

This PR changes the affected test to just look for ArithmeticException rather than specifically java.lang.ArithmeticException. This is the same approach used in the other tests.

@andygrove andygrove added this to the Apr 4 - Apr 15 milestone Apr 8, 2022
@andygrove andygrove self-assigned this Apr 8, 2022
@andygrove andygrove changed the title WIP: Temporarily ignore tests that fail with Spark 3.3 Temporarily ignore tests that fail with Spark 3.3 Apr 8, 2022
@andygrove andygrove marked this pull request as ready for review April 8, 2022 20:17
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove andygrove changed the title Temporarily ignore tests that fail with Spark 3.3 Fix test failure with Spark 3.3 by looking for less specific error message Apr 8, 2022
@andygrove
Copy link
Contributor Author

build

@@ -788,7 +788,7 @@ def test_div_overflow_exception_when_ansi(expr, ansi_enabled):
assert_gpu_and_cpu_error(
df_fun=lambda spark: _get_div_overflow_df(spark, expr).collect(),
conf=ansi_conf,
error_message='java.lang.ArithmeticException: Overflow in integral divide')
error_message='ArithmeticException: Overflow in integral divide')
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the point to ensure we're throwing the same exception? As such I'd rather see this test check for the correct exception class based on the Spark version -- if Spark 3.3 throws a different exception for this, then our plugin should throw the same class for the same type of exception on Spark 3.3.

I'm OK if we want to do this as a followup to refine the test if we aren't already doing the correct behavior now. If we are, then this should choose a different message to check based on which Spark we're using as is done in other tests checking exception types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's a good point. I filed #5196 as a follow-on.

@sameerz sameerz added the audit_3.3.0 Audit related tasks for 3.3.0 label Apr 10, 2022
@jlowe jlowe merged commit 32ecd15 into NVIDIA:branch-22.06 Apr 11, 2022
@andygrove andygrove deleted the temp-workadound-issue-5182 branch May 17, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit_3.3.0 Audit related tasks for 3.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Spark 3.3 integration tests arithmetic_ops_test.py::test_div_overflow_exception_when_ansi failures
4 participants