You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2023/09/12 13:41:25 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
zhengruifeng opened a new pull request, #42891:
URL: https://github.com/apache/spark/pull/42891
### What changes were proposed in this pull request?
- Introduce a parameterized decorator for test retry
- Retry `test_read_images` if it fails
### Why are the changes needed?
previously, we used `utils.condition` to retry flaky tests, e.g. https://github.com/apache/spark/commit/745ed93fe451b3f9e8148b06356c28b889a4db5a
however, it needs to modify the test body.
To minimize the changes, I'd like to add a decorator for test retry.
recently, I see `test_read_images` fail multiple times, e.g. https://github.com/apache/spark/actions/runs/6155818962/job/16703369881
### Does this PR introduce _any_ user-facing change?
no, test-only
### How was this patch tested?
manually test random case:
```
@retry(maxTries=50)
def test_rand(self):
import random
self.assertTrue(random.random() < 0.1)
```
in the log:
```
The 1-th attempt failed, due to False is not true!
The 2-th attempt failed, due to False is not true!
The 3-th attempt failed, due to False is not true!
The 4-th attempt failed, due to False is not true!
The 5-th attempt failed, due to False is not true!
The 6-th attempt failed, due to False is not true!
The 7-th attempt failed, due to False is not true!
The 8-th attempt failed, due to False is not true!
The 9-th attempt failed, due to False is not true!
The 10-th attempt failed, due to False is not true!
The 11-th attempt failed, due to False is not true!
ok (11.055s)
```
### Was this patch authored or co-authored using generative AI tooling?
no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323880837
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
hmm, maybe we can add an additional parameter to treat returned `None` as `True`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323875813
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
I see. Can you make `eventually` to work as a decorator too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323855018
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
I see, I mentioned it in the PR description.
I think it will be more easy to use if we make it a decorator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323879995
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
Can we reuse `eventually` here?
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
Can we reuse `eventually` here to avoid duplication?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng closed pull request #42891: [SPARK-45135][PYTHON][TESTS] Make `utils.eventually` a parameterized decorator
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #42891: [SPARK-45135][PYTHON][TESTS] Make `utils.eventually` a parameterized decorator
URL: https://github.com/apache/spark/pull/42891
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #42891: [SPARK-45135][PYTHON][TESTS] Make `utils.eventually` a parameterized decorator
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42891:
URL: https://github.com/apache/spark/pull/42891#issuecomment-1718474420
In the download log artifact, the added 3 UT:
```
<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="pyspark.tests.test_util.UtilTests-20230913111701" tests="6" file="pyspark/tests/test_util.py" time="7.334" timestamp="2023-09-13T11:17:09" failures="0" errors="0" skipped="0">
<testcase classname="pyspark.tests.test_util.UtilTests" name="test_eventually_decorator" time="4.326" timestamp="2023-09-13T11:17:06" file="python/pyspark/tests/test_util.py" line="87">
<system-out><![CDATA[
Attempt #1 failed!
False is not true
Attempt #2 failed!
False is not true
Attempt #3 failed!
False is not true
Attempt #4 failed!
False is not true
Attempt #5 failed!
False is not true
Attempt #6 failed!
False is not true
Attempt #7 failed!
False is not true
Attempt #8 failed!
False is not true
Attempt #9 failed!
False is not true
Attempt #10 failed!
False is not true
Attempt #11 failed!
False is not true
Attempt #12 failed!
False is not true
Attempt #13 failed!
False is not true
]]></system-out>
</testcase>
<testcase classname="pyspark.tests.test_util.UtilTests" name="test_eventually_function" time="0.610" timestamp="2023-09-13T11:17:06" file="python/pyspark/tests/test_util.py" line="93">
<system-out><![CDATA[
Attempt #1 failed!
False is not true
Attempt #2 failed!
False is not true
Attempt #3 failed!
False is not true
Attempt #4 failed!
False is not true
]]></system-out>
</testcase>
<testcase classname="pyspark.tests.test_util.UtilTests" name="test_eventually_lambda" time="0.602" timestamp="2023-09-13T11:17:07" file="python/pyspark/tests/test_util.py" line="101">
<system-out><![CDATA[
Attempt #1 failed!
False is not true
Attempt #2 failed!
False is not true
Attempt #3 failed!
False is not true
Attempt #4 failed!
False is not true
Attempt #5 failed!
False is not true
Attempt #6 failed!
False is not true
Attempt #7 failed!
False is not true
Attempt #8 failed!
False is not true
Attempt #9 failed!
False is not true
Attempt #10 failed!
False is not true
]]></system-out>
</testcase>
<testcase classname="pyspark.tests.test_util.UtilTests" name="test_find_spark_home" time="0.595" timestamp="2023-09-13T11:17:08" file="python/pyspark/tests/test_util.py" line="78"/>
<testcase classname="pyspark.tests.test_util.UtilTests" name="test_parsing_version_string" time="0.617" timestamp="2023-09-13T11:17:08" file="python/pyspark/tests/test_util.py" line="73"/>
<testcase classname="pyspark.tests.test_util.UtilTests" name="test_py4j_str" time="0.584" timestamp="2023-09-13T11:17:09" file="python/pyspark/tests/test_util.py" line="66"/>
</testsuite>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323851428
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
we have a similar utility `eventually` at `pyspark.testing.utils`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323879316
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
it is a bit difference that `eventually` requires the function return `True` if succeed, so I will make it a different decorator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Introduce a parameterized decorator for test retry
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1323878148
##########
python/pyspark/testing/utils.py:
##########
@@ -115,6 +115,36 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
)
+def retry(maxTries=10, interval=1.0):
Review Comment:
sound good, let me have a try
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42891: [SPARK-45135][PYTHON][TESTS] Make `utils.eventually` a parameterized decorator
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42891:
URL: https://github.com/apache/spark/pull/42891#discussion_r1325205015
##########
python/pyspark/testing/utils.py:
##########
@@ -80,7 +83,7 @@ def eventually(condition, timeout=30.0, catch_assertions=False):
----------
condition : function
Function that checks for termination conditions. condition() can return:
- - True: Conditions met. Return without error.
+ - True or None: Conditions met. Return without error.
Review Comment:
@HyukjinKwon I made a change here, so that we don't need `return True` for most cases in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #42891: [SPARK-45135][PYTHON][TESTS] Make `utils.eventually` a parameterized decorator
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42891:
URL: https://github.com/apache/spark/pull/42891#issuecomment-1718493340
merged to master
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org