Skip to content

[SPARK-XXXXX][SQL][TESTS] Drop redundant BeforeAndAfterEach and SharedSparkSession mixins#55819

Draft
zhengruifeng wants to merge 1 commit into
apache:masterfrom
zhengruifeng:cleanup-beforeandaftereach-mixin
Draft

[SPARK-XXXXX][SQL][TESTS] Drop redundant BeforeAndAfterEach and SharedSparkSession mixins#55819
zhengruifeng wants to merge 1 commit into
apache:masterfrom
zhengruifeng:cleanup-beforeandaftereach-mixin

Conversation

@zhengruifeng
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Two related cleanups in sql/core test suites:

  1. Drop redundant with BeforeAndAfterEach (5 files). SharedSparkSession transitively mixes in BeforeAndAfterEach via SharedSparkSessionBase, so listing it again is redundant:

    • StreamingSourceEvolutionSuite (the explicit mixin sat after StreamTest, which already chains to SharedSparkSession)
    • RocksDBTimestampEncoderOperationsSuite
    • ResolveChangelogTableNetChangesTestsBase (trait)
    • ResolveChangelogTablePostProcessingSuite
    • ResolveChangelogTableStreamingPostProcessingSuite
  2. Drop redundant with SharedSparkSession after StreamTest (5 files). StreamTest extends SharedSparkSession with TimeLimits, so listing SharedSparkSession again adds nothing:

    • ForeachWriterSuite
    • FileStreamSourceTest (the abstract base used by FileStreamSourceSuite / FileStreamSourceStressTestSuite)
    • TextSocketStreamSuite
    • OperatorStateMetadataSuite
    • RocksDBCheckpointFailureInjectionSuite

Unused imports of org.scalatest.BeforeAndAfterEach and org.apache.spark.sql.test.SharedSparkSession are removed alongside.

Suites that mix in the older org.scalatest.BeforeAndAfter (singular, e.g. ForeachWriterSuite, SaveLoadSuite) keep it - that is a distinct trait with its own after { ... } block syntax, not a redundant alias for BeforeAndAfterEach. SaveLoadSuite is intentionally untouched: its base DataSourceTest extends QueryTest (not SharedSparkSession), so the explicit with SharedSparkSession there is load-bearing.

Why are the changes needed?

SharedSparkSession already extends QueryTest with SharedSparkSessionBase, and SharedSparkSessionBase already mixes in BeforeAndAfterEach. StreamTest already extends SharedSparkSession. Repeating these in extends clauses adds noise and gives the false impression they are independently required, which makes it easy for the pattern to spread to new suites.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests; build/sbt sql/Test/compile passes cleanly on the affected sources.

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

Generated-by: Claude Code

…dSparkSession mixins

SharedSparkSession transitively extends BeforeAndAfterEach (via SharedSparkSessionBase), so
explicit `with BeforeAndAfterEach` alongside SharedSparkSession is redundant.

StreamTest extends SharedSparkSession, so explicit `with SharedSparkSession` after StreamTest
is redundant.

This change drops:
- `with BeforeAndAfterEach` from 5 suites whose base already provides it: StreamingSourceEvolutionSuite,
  RocksDBTimestampEncoderOperationsSuite, ResolveChangelogTableNetChangesTestsBase,
  ResolveChangelogTablePostProcessingSuite, ResolveChangelogTableStreamingPostProcessingSuite.
- `with SharedSparkSession` from 5 StreamTest-based suites: ForeachWriterSuite, FileStreamSourceTest,
  TextSocketStreamSuite, OperatorStateMetadataSuite, RocksDBCheckpointFailureInjectionSuite.

Unused `BeforeAndAfterEach` / `SharedSparkSession` imports are also removed. Suites that mix in the
older `org.scalatest.BeforeAndAfter` (singular) trait keep it - that's a distinct API, not redundant.

Generated-by: Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant