Yikun commented on a change in pull request #32867:
URL: https://github.com/apache/spark/pull/32867#discussion_r654522707
##########
File path: dev/sparktestsupport/modules.py
##########
@@ -58,14 +95,19 @@ def __init__(self, name, dependencies, source_file_regexes,
build_profile_flags=
is not explicitly changed.
:param should_run_r_tests: If true, changes in this module will
trigger all R tests.
:param should_run_build_tests: If true, changes in this module will
trigger build tests.
+ :param python_test_paths: A set of python test paths to be discovered
+ :param python_excluded_tests: A set of excluded Python tests
"""
self.name = name
self.dependencies = dependencies
self.source_file_prefixes = source_file_regexes
self.sbt_test_goals = sbt_test_goals
self.build_profile_flags = build_profile_flags
self.environ = environ or {}
- self.python_test_goals = python_test_goals
+ discovered_goals = _discover_python_unittests(python_test_paths)
+ # Final goals = Manual specified goals + Discoverd goals - Excluded
goals
+ all_goals = set(python_test_goals) | set(discovered_goals)
+ self.python_test_goals = sorted(list(set(all_goals) -
set(python_excluded_tests)))
Review comment:
Yes, great suggestion, it makes module implementation more clean.
If we want to **only** discover the slow tests
```
python_test_goals = [
...
] + _discover_python_unittests("pyspark/tests", discover_slow=True)
```
otherwise will skip slow tests.
I complete it in my next commit. : )
##########
File path: dev/sparktestsupport/modules.py
##########
@@ -488,12 +495,10 @@ def __hash__(self):
python_test_goals=[
# doctests
"pyspark.streaming.util",
Review comment:
Frankly, not yet, but I'd like to start to do some investigation. This
unittestPR is started when I forgot many times to add the unittests in module.
^_^
And the `doctests` discover is a bit special, it's not subclass of doctest
(just like the unitest), so looks like there are 2 potential solution:
1. Flag based. Add some flag like `_doc_test = True` in every test we want
to test.
2. Special name based. Discover which module contains `_doctest` method (we
should rename _test to _doctest to make doc test name unique).
##########
File path: dev/sparktestsupport/modules.py
##########
@@ -488,12 +495,10 @@ def __hash__(self):
python_test_goals=[
# doctests
"pyspark.streaming.util",
Review comment:
BTW, do you think the footer method (excute doctest or unitests in
__main__ with repeat code in many files) in python code is a good way enough?
Do we consider do some re-arch (or to say, already have some work)on testing
framework?
##########
File path: dev/sparktestsupport/modules.py
##########
@@ -488,12 +495,10 @@ def __hash__(self):
python_test_goals=[
# doctests
"pyspark.streaming.util",
Review comment:
BTW, I'm not sure here is the good place to discussion. : )
Do you think the footer method (excute doctest or unitests in __main__ with
repeat code in many files) in python code is a good way enough? Do we consider
do some re-arch (or to say, already have some work) on testing framework?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
