test: Run integration tests against v2 as well#657
Conversation
|
@CodiumAI-Agent |
nightscape
force-pushed
the
v2-integration-tests
branch
from
00e4ab3 to
5999b76
Compare
PR Analysis
PR Feedback
How to use
|
nightscape
force-pushed
the
v2-integration-tests
branch
from
5999b76 to
5929991
Compare
nightscape
force-pushed
the
v2-integration-tests
branch
2 times, most recently
from
ea6064d to
3dc5e63
Compare
nightscape
force-pushed
the
main
branch
3 times, most recently
from
6b58ec4 to
6866cb1
Compare
nightscape
force-pushed
the
v2-integration-tests
branch
from
3dc5e63 to
d6e5746
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Scala integration test suite to run the same set of tests against multiple Excel datasource implementations (intended: v1 vs v2), enabling comparative coverage across implementations.
Changes:
- Refactors
IntegrationSuiteinto an abstract base suite parameterized by a datasourceimplementationstring. - Switches the core write/read helper from
.excel(...)helpers to explicit.format(implementation)with options. - Adds two concrete ScalaTest suites (
IntegrationSuiteV1,IntegrationSuiteV2) to run the same tests against different implementations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val writer = df.write | ||
| .format(implementation) | ||
| .option("dataAddress", s"'$sheetName'!A1") | ||
| .option("header", header) | ||
| .mode(saveMode) | ||
| val configuredWriter = | ||
| Map("dataAddress" -> dataAddress).foldLeft(writer) { | ||
| case (wri, (key, Some(value))) => wri.option(key, value) | ||
| case (wri, _) => wri | ||
| } | ||
| configuredWriter.save(theFileName) | ||
|
|
||
| val reader = spark.read.excel(dataAddress = s"'$sheetName'!A1", header = header) | ||
| val reader = spark.read.format(implementation).option("dataAddress", s"'$sheetName'!A1").option("header", header) | ||
| val configuredReader = Map( |
There was a problem hiding this comment.
The suite is now parameterized by implementation, but there are still direct .excel(...) reads later in this file that hardcode the v1 provider (via the package-level implicit) and will bypass the selected implementation. That means IntegrationSuiteV2 won't consistently exercise the v2 datasource. Consider switching the remaining .excel(...) usages to spark.read.format(implementation) (and setting the same options), so both suites truly test the chosen implementation end-to-end.
| runTests(maxRowsInMemory = Some(1), maxByteArraySize = Some(100000000)) | ||
| } | ||
|
|
||
| class IntegrationSuiteV1 extends IntegrationSuite("com.crealytics.spark.excel") |
There was a problem hiding this comment.
IntegrationSuiteV1 uses format("com.crealytics.spark.excel"), but this repository doesn't appear to include or depend on the com.crealytics.spark.excel.DefaultSource provider. As written, the V1 suite will fail at runtime when Spark tries to resolve that datasource. Either add the crealytics spark-excel dependency to the test classpath, or change the v1 implementation string to this project’s v1 provider (e.g., "dev.mauch.spark.excel") if the intent is to test the built-in v1 datasource.
| class IntegrationSuiteV1 extends IntegrationSuite("com.crealytics.spark.excel") | |
| class IntegrationSuiteV1 extends IntegrationSuite("dev.mauch.spark.excel") |
- Fix V1 format name: com.crealytics.spark.excel → dev.mauch.spark.excel - Replace all .excel() convenience method calls with .format(implementation) to properly test both V1 and V2 code paths - Set ignoreNullable=true in assertDataFrameApproximateEquals since V2 returns all fields as nullable - Avoid .rdd conversion in assertEqualAfterInferringTypes which causes encoding errors on Spark 4 - Skip append-to-existing-file tests for V2 since FileDataSourceV2 writes to directories instead of single files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add missing "header" option to "reads files with missing cells" test (V2 requires it explicitly) - Treat null and empty string as equivalent in row comparison since V2 reads empty cells as null while V1 reads them as empty string Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No description provided.