Fixed Failing tests in arithmetic_ops_tests for Spark 4.0.0 [databricks] #11044
Conversation
Signed-off-by: Raza Jafri <[email protected]>
razajafri
changed the title
|
@revans2 do you have any more questions that I can help answer? |
|
I have modified this PR to disable ansi mode per test instead of a global conf. The advantage is that we can take that off if we need to once we modify tests to handle the ansi mode. |
|
Build |
|
@mythrocks Can you PTAL |
|
@revans2 can you PTAL |
|
P.S. Using an annotation makes this very neat. Good idea. |
|
build |
|
I am looking into the premerge failure. |
|
build |
|
@mythrocks I think I found the problem. It was because I was setting the ansi mode to True if it wasn't set in the test which caused problem in other tests |
|
build |
|
|
||
| def is_allowing_any_non_gpu(): | ||
| return _allow_any_non_gpu | ||
|
|
||
| def get_non_gpu_allowed(): | ||
| return _non_gpu_allowed | ||
|
|
||
| def get_per_test_ansi_mode(): |
There was a problem hiding this comment.
I wonder if the logic problem could have been avoided if the names of the variable and function were better. Note that the annotation is a "negative" (ansi_mode_disabled).
| def get_per_test_ansi_mode(): | |
| def is_per_test_ansi_mode_enabled(): |
Likewise, the variable should probably be named _per_test_ansi_mode_enabled.
It is confusing to read, as it currently stands.
| @@ -16,6 +16,7 @@ | |||
|
|
|||
| allow_non_gpu_databricks = pytest.mark.allow_non_gpu_databricks | |||
| allow_non_gpu = pytest.mark.allow_non_gpu | |||
| ansi_mode_disabled = pytest.mark.ansi_mode_disabled | |||
There was a problem hiding this comment.
Now that I look at this, it would be better to use an imperative/active voice for directives:
| ansi_mode_disabled = pytest.mark.ansi_mode_disabled | |
| disable_ansi_mode = pytest.mark.disable_ansi_mode |
Notice that all the other annotations are in active voice.
There was a problem hiding this comment.
P.S. Apologies for the churn.
|
build |
razajafri
force-pushed
the
SP-11006-arithmetic_tests
branch
from
91ad87b
to
08e4eab
Compare
|
build |
|
Premerge failed sure to insufficient capacity of databricks clusters with the required conf |
|
Build |
|
Looks like it's the cache tests that are still failing: Odd. This should have been fixed by 0a6fbf3. I'll review the logic again, and kick off another build. |
This handles cases like `cache_test.py` which should run with the default conf for `spark.sql.ansi.enabled`.
|
Build |
…ks] (NVIDIA#11044) * Fixed arithmetic_ops_tests * Signing off Signed-off-by: Raza Jafri <[email protected]> * Added a mechanism to add ansi mode per test * Reverted unnecessary change to spark_init_internal.py * Corrected the year in the licence * Only set ansi conf to false when ansi_mode_disabled is set * Addressed review comments * Fixed the method name * Update integration_tests/src/main/python/conftest.py This handles cases like `cache_test.py` which should run with the default conf for `spark.sql.ansi.enabled`. --------- Signed-off-by: Raza Jafri <[email protected]> Co-authored-by: MithunR <[email protected]>
This PR fixes arithmetic_ops_tests failures
We are explicitly setting the
spark.sql.ansi.enabledto false as that is how our tests are written.There are various changes to the Exception messages and types for some methods that were also fixed.
fixes #11006