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

[SPARK-46812][PYTHON][TESTS][FOLLOWUP] Check should_test_connect and pyarrow to skip tests #45880

Closed
wants to merge 2 commits into from
Closed

[SPARK-46812][PYTHON][TESTS][FOLLOWUP] Check should_test_connect and pyarrow to skip tests #45880

wants to merge 2 commits into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Apr 4, 2024 •
edited
Loading

What changes were proposed in this pull request?

This is a follow-up of SPARK-46812 to skip the tests more robustly and to recover PyPy CIs.

Why are the changes needed?

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manually tests with pandas and without pyarrow.

$ pip3 freeze | grep pyarrow
$ pip3 freeze | grep pandas
pandas==2.2.1
pandas-stubs==1.2.0.53

$ python/run-tests --modules=pyspark-resource --parallelism=1 --python-executables=python3.10
Running PySpark tests. Output is in /Users/dongjoon/APACHE/spark-merge/python/unit-tests.log
Will test against the following Python executables: ['python3.10']
Will test the following Python modules: ['pyspark-resource']
python3.10 python_implementation is CPython
python3.10 version is: Python 3.10.13
Starting test(python3.10): pyspark.resource.profile (temp output: /Users/dongjoon/APACHE/spark-merge/python/target/db9cb886-2698-49d9-a663-9b8bea79caba/python3.10__pyspark.resource.profile__8mg46xru.log)
Finished test(python3.10): pyspark.resource.profile (1s)
Starting test(python3.10): pyspark.resource.tests.test_connect_resources (temp output: /Users/dongjoon/APACHE/spark-merge/python/target/53f979bd-1073-41e6-99ba-8e787edc415b/python3.10__pyspark.resource.tests.test_connect_resources__hrgrs5sk.log)
Finished test(python3.10): pyspark.resource.tests.test_connect_resources (0s) ... 1 tests were skipped
Starting test(python3.10): pyspark.resource.tests.test_resources (temp output: /Users/dongjoon/APACHE/spark-merge/python/target/2b06c671-0199-4827-a0e5-f852a28313fd/python3.10__pyspark.resource.tests.test_resources__jis6mk9a.log)
Finished test(python3.10): pyspark.resource.tests.test_resources (2s) ... 1 tests were skipped
Tests passed in 4 seconds

Skipped tests in pyspark.resource.tests.test_connect_resources with python3.10:
      test_profile_before_sc_for_connect (pyspark.resource.tests.test_connect_resources.ResourceProfileTests) ... skip (0.002s)

Skipped tests in pyspark.resource.tests.test_resources with python3.10:
      test_profile_before_sc_for_sql (pyspark.resource.tests.test_resources.ResourceProfileTests) ... skip (0.001s)

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the PYTHON label Apr 4, 2024
dongjoon-hyun
dongjoon-hyun commented Apr 4, 2024
View reviewed changes
python/pyspark/resource/tests/test_connect_resources.py


@unittest.skipIf(not have_pandas, pandas_requirement_message)
@unittest.skipIf(not should_test_connect, connect_requirement_message)
Copy link
Member Author

Choose a reason for hiding this comment

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

should_test_connect covers more edge cases than have_pandas.

dongjoon-hyun
dongjoon-hyun commented Apr 4, 2024
View reviewed changes
python/pyspark/resource/tests/test_resources.py
@@ -72,7 +78,10 @@ def assert_request_contents(exec_reqs, task_reqs):
assert_request_contents(rp3.executorResources, rp3.taskResources)
sc.stop()

@unittest.skipIf(not have_pandas, pandas_requirement_message)
@unittest.skipIf(
not have_pandas or not have_pyarrow,
Copy link
Member Author

Choose a reason for hiding this comment

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

This test has df.mapInArrow(lambda x: x, df.schema, False, rp).collect() too.

@xinrong-meng
Copy link
Member

LGTM thank you!

@dongjoon-hyun
Copy link
Member Author

Thank you, @xinrong-meng !

@dongjoon-hyun
Copy link
Member Author

Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-46812-2 branch April 4, 2024 19:49
@dongjoon-hyun
Copy link
Member Author

For the record, it's green now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xinrong-meng xinrong-meng xinrong-meng approved these changes

Assignees
No one assigned
Labels
PYTHON
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dongjoon-hyun @xinrong-meng