Core, Spark: Migrate tests that depend on ScanTestBase to JUnit5 #9416

chinmay-bhat · 2024-01-05T07:58:14Z

This PR migrates tests that depend on SparkDistributedDataScanTestBase to JUnit5.
SparkDistributedDataScanTestBase inherits from classes that ultimately inherit from TableTestBase.

TableTestBase uses JUnit4 imports and is the base class for atleast 90+ classes across flink, core, data, and spark. So I created ParameterizedTableTestBase, a version of TableTestBase migrated to JUnit5. This is similar to #9076

Issue: #9086

chinmay-bhat · 2024-01-05T08:27:28Z

Based on the test failures, older spark versions depend on core files like DeleteFileIndexTestBase, ParameterizedTableTestBase, DataTableScanTestBase,ScanPlanningAndReportingTestBase, which have been updated to JUnit5 in this PR.

A possible solution would be to migrate the affected files in older versions to JUnit5 as well. That would be a task bigger than this PRs scope. Another solution would be to create JUnit5 versions of the above core files, but I'm not keen on creating unnecessary duplicates.

@nastra @Fokko
I faced a similar problem in #9382. Any advise would be great! :)

core/src/test/java/org/apache/iceberg/ScanTestBase.java

This reverts commit 2119e60.

core/src/test/java/org/apache/iceberg/TestBase.java

spark/v3.5/spark/src/test/java/org/apache/iceberg/SparkDistributedDataScanTestBase.java

spark/v3.5/spark/src/test/java/org/apache/iceberg/TestSparkDistributedDataScanDeletes.java

spark/v3.5/spark/src/test/java/org/apache/iceberg/TestSparkDistributedDataScanReporting.java

data/src/test/java/org/apache/iceberg/io/TestGenericSortedPosDeleteWriter.java

core/src/test/java/org/apache/iceberg/FilterFilesTestBase.java

core/src/test/java/org/apache/iceberg/TestLocalFilterFiles.java

core/src/test/java/org/apache/iceberg/ScanPlanningAndReportingTestBase.java

core/src/test/java/org/apache/iceberg/TestBase.java

core/src/test/java/org/apache/iceberg/TestLocalFilterFiles.java

core/src/test/java/org/apache/iceberg/DeleteFileIndexTestBase.java

This reverts commit ade28f5.

chinmay-bhat · 2024-01-15T12:00:50Z

We have files that inherit from TestBase used in older Spark versions that are causing CI failure. Should I move them to JUnit5 like we did in #9382 ?

nastra · 2024-01-15T13:51:55Z

We have files that inherit from TestBase used in older Spark versions that are causing CI failure. Should I move them to JUnit5 like we did in #9382 ?

Yes that would probably be the best here

chinmay-bhat · 2024-01-15T16:14:54Z

not sure why the one CI test is failing, all tests pass locally for me. Can you rerun the CI?

nastra

LGTM, thanks @chinmay-bhat for getting this done!

…che#9416)

github-actions bot added spark core labels Jan 5, 2024

nastra reviewed Jan 8, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/ScanTestBase.java Outdated Show resolved Hide resolved

chinmay-bhat added 4 commits January 9, 2024 09:40

create ParameterizedTableTestBase.java

2119e60

migrate tests to JUnit5

3121b37

Revert "create ParameterizedTableTestBase.java"

22cafd1

This reverts commit 2119e60.

extend TestBase

5bf3026

chinmay-bhat force-pushed the junit5-spark-distributed-data-scan branch from 8b2c13c to 5bf3026 Compare January 9, 2024 16:40

github-actions bot added the data label Jan 9, 2024