wombatu-kun commented on code in PR #16677:
URL: https://github.com/apache/iceberg/pull/16677#discussion_r3353055707
##########
api/src/test/java/org/apache/iceberg/TestHelpers.java:
##########
@@ -74,12 +74,26 @@ public static <T> Stream<Arguments> serializers() {
private TestHelpers() {}
public static final int MAX_FORMAT_VERSION = 4;
+
+ // The highest format version covered by the default version-parameterized
test ranges below.
+ // While a new format version is being incubated, this lags
MAX_FORMAT_VERSION so that the
+ // existing parameterized test suite is not pinned to an incomplete code
path. Bump this to
+ // MAX_FORMAT_VERSION when the incubating version's read/write code paths
cover every feature
+ // exercised by the parameterized suite.
+ public static final int MAX_PRODUCTION_VERSION = 3;
Review Comment:
One nuance on narrowing all three ranges uniformly: the incubation rationale
(incomplete v4 read/write paths cause spurious failures) applies to tests that
read/write data, but a few `@FieldSource(ALL_VERSIONS)` consumers are pure
metadata/JSON round-trips that never touch those paths. For example
`TestLoadTableResponseParser.roundTripSerdeV3andHigher` is parameterized over
`ALL_VERSIONS` with `assumeThat(formatVersion).isGreaterThanOrEqualTo(3)`, so
today it already runs (and passes in CI) for both v3 and v4; after this change
it silently drops to v3-only.
Is dropping v4 from these serde-only tests intended, or worth keeping an
explicit v4 case for them (the way `TestManifestWriterVersions` keeps dedicated
`V4_FORMATS` methods alongside its `ALL_VERSIONS` ones)? Not blocking - just
flagging that the blanket narrowing also removes v4 coverage that isn't on the
incomplete read/write path.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
