[SPARK-56863][PYTHON][PS][TESTS] Drop redundant SQLTestUtils#55875
Open
zhengruifeng wants to merge 3 commits into
Open
[SPARK-56863][PYTHON][PS][TESTS] Drop redundant SQLTestUtils#55875zhengruifeng wants to merge 3 commits into
SQLTestUtils#55875zhengruifeng wants to merge 3 commits into
Conversation
…usedMixedTestCase`
`ReusedMixedTestCase` was declared as
class ReusedMixedTestCase(ReusedConnectTestCase, SQLTestUtils):
but its parent `ReusedConnectTestCase` already mixes in `SQLTestUtils`:
class ReusedConnectTestCase(PySparkBaseTestCase, SQLTestUtils, PySparkErrorTestUtils):
so listing `SQLTestUtils` again on `ReusedMixedTestCase` is redundant. The MRO and the inherited helper methods (`sql_conf`, `table`, `temp_view`, ...) are unchanged after the drop:
MRO before/after: ['ReusedMixedTestCase', 'ReusedConnectTestCase',
'PySparkBaseTestCase', 'TestCase', 'SQLTestUtils',
'PySparkErrorTestUtils', 'object']
The `from pyspark.testing.sqlutils import SQLTestUtils` import is kept because `ReusedConnectTestCase` still uses it.
Generated-by: Claude opus-4-7
zhengruifeng
changed the title
SQLTestUtils mixin from ReusedMixedTestCaseSQLTestUtils mixin from ReusedMixedTestCase
…nSparkTestCase`-based test classes
`PandasOnSparkTestCase` already chains to `SQLTestUtils`:
PandasOnSparkTestCase -> ReusedSQLTestCase -> SQLTestUtils
so explicitly mixing `SQLTestUtils` in again on top of `PandasOnSparkTestCase` (and an `XxxMixin` body trait) is dead weight. This commit drops the redundant `SQLTestUtils` mixin -- and the now-unused `from pyspark.testing.sqlutils import SQLTestUtils` import -- from 139 test classes under `python/pyspark/pandas/tests/` that follow the pattern:
class FooTests(
FooTestsMixin,
PandasOnSparkTestCase,
SQLTestUtils, # <-- redundant
):
pass
The MRO, inherited helper methods (`sql_conf`, `table`, `temp_view`, ...), and `unittest.TestCase` subclass relationship are all unchanged after the drop.
Classes elsewhere that legitimately use `SQLTestUtils` as their main TestCase-adjacent base (e.g. `QueryExecutionListenerTests` in `python/pyspark/sql/tests/test_listener.py`, which mixes `unittest.TestCase` and `SQLTestUtils` directly) are not touched.
Generated-by: Claude opus-4-7
zhengruifeng
changed the title
SQLTestUtils mixin from ReusedMixedTestCaseSQLTestUtils mixin from ReusedMixedTestCase and 139 PandasOnSparkTestCase-based test classes
zhengruifeng
changed the title
SQLTestUtils mixin from ReusedMixedTestCase and 139 PandasOnSparkTestCase-based test classesSQLTestUtils mixin from ReusedMixedTestCase and 139 PandasOnSparkTestCase-based test classes
zhengruifeng
changed the title
SQLTestUtils mixin from ReusedMixedTestCase and 139 PandasOnSparkTestCase-based test classesSQLTestUtils
…Utils` drop
Three test classes ended up with their bases split across three lines after dropping `SQLTestUtils`:
class FooTests(
FooTestsMixin, PandasOnSparkTestCase[, TestUtils]
):
pass
`black` (CI's formatter) wants the bases on one line now that they fit under the 100-char limit:
class FooTests(FooTestsMixin, PandasOnSparkTestCase[, TestUtils]):
pass
Files:
- `python/pyspark/pandas/tests/diff_frames_ops/test_setitem_series.py`
- `python/pyspark/pandas/tests/test_frame_spark.py`
- `python/pyspark/pandas/tests/test_indexops_spark.py`
Generated-by: Claude opus-4-7
zhengruifeng
changed the title
SQLTestUtilsSQLTestUtils
zhengruifeng
changed the title
SQLTestUtilsSQLTestUtils
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Drop the redundant
SQLTestUtilsmixin from Python test classes whose primary base already chains toSQLTestUtils. Two cleanups in this PR.1.
ReusedMixedTestCase(python/pyspark/testing/connectutils.py)Its parent
ReusedConnectTestCasealready mixes inSQLTestUtils:The
from pyspark.testing.sqlutils import SQLTestUtilsimport is kept becauseReusedConnectTestCase(still in the same file) uses it.2. 139
PandasOnSparkTestCase-based classes underpython/pyspark/pandas/tests/PandasOnSparkTestCasealready chains toSQLTestUtils:so explicitly mixing
SQLTestUtilsin again on top ofPandasOnSparkTestCase(and anXxxMixinbody trait) is dead weight. This PR drops the redundantSQLTestUtilsmixin -- and the now-unusedfrom pyspark.testing.sqlutils import SQLTestUtilsimport -- from 139 test classes following the pattern:The MRO, inherited helper methods (
sql_conf,table,temp_view, ...), andunittest.TestCasesubclass relationship are all unchanged after the drop. Verified across a representative sample (ReusedMixedTestCase,NamespaceTests,NumPyCompatTests,UnionTests,SparkFrameMethodsTests):TestCaseSQLTestUtilssql_confresolvesClasses elsewhere that legitimately use
SQLTestUtilsas their mainTestCase-adjacent base (e.g.QueryExecutionListenerTestsinpython/pyspark/sql/tests/test_listener.py, which mixesunittest.TestCaseandSQLTestUtilsdirectly) are not touched.Why are the changes needed?
Cleanup -- same spirit as the recently merged [SPARK-56841][SQL][TESTS] (
Drop redundant BeforeAndAfterEach and SharedSparkSession mixins) but on the Python side.Does this PR introduce any user-facing change?
No. Test-infra-only change.
How was this patch tested?
Existing tests. MRO /
issubclass/ inherited-method checks (above) confirm the behavior of affected classes is unchanged.py_compileover a sample of the edited files succeeds.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude opus-4-7