yihua commented on code in PR #13687:
URL: https://github.com/apache/hudi/pull/13687#discussion_r2265066113
##########
hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/table/upgrade/TestUpgradeDowngrade.java:
##########
@@ -163,6 +191,88 @@ public void testAutoUpgradeDisabled(HoodieTableVersion
originalVersion) throws E
LOG.info("Auto-upgrade disabled test passed for version {}",
originalVersion);
}
+ @ParameterizedTest
+ @MethodSource("writeTableVersionTestCases")
+ public void testAutoUpgradeWithWriteTableVersionConfiguration(
+ Integer writeTableVersion, HoodieTableVersion expectedVersion, String
description) throws Exception {
Review Comment:
```suggestion
Option<HoodieTableVersion> writeTableVersion, HoodieTableVersion
expectedVersion, String description) throws Exception {
```
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/table/upgrade/TestUpgradeDowngradeLegacy.java:
##########
@@ -709,10 +710,11 @@ void testNeedsUpgrade() {
assertTrue(shouldUpgrade);
// assert upgrade for table version 8 from table version 6 with auto
upgrade set to false
+ // should return false
when(writeConfig.autoUpgrade()).thenReturn(false);
shouldUpgrade = new UpgradeDowngrade(metaClient, writeConfig, context,
null)
.needsUpgrade(HoodieTableVersion.EIGHT);
- assertTrue(shouldUpgrade);
+ assertFalse(shouldUpgrade);
Review Comment:
Have you checked the existing test cases cover all combinations for
`needsUpgrade`?
##########
hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/table/upgrade/TestUpgradeDowngrade.java:
##########
@@ -304,6 +429,27 @@ private static Stream<Arguments>
upgradeDowngradeVersionPairs() {
);
}
+ /**
+ * Test cases for auto-upgrade with different hoodie.write.table.version
configurations.
+ * Each case starts with table version SIX and tests different write table
version settings.
+ * Test params are: Integer writeTableVersion, HoodieTableVersion
expectedVersion, String description
+ */
+ private static Stream<Arguments> writeTableVersionTestCases() {
Review Comment:
nit: put this method before
`testAutoUpgradeWithWriteTableVersionConfiguration` so it's easier to check the
testing arguments?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
