rdblue commented on a change in pull request #1101:
URL: https://github.com/apache/iceberg/pull/1101#discussion_r438377910
##########
File path: core/src/test/java/org/apache/iceberg/TestMergeAppend.java
##########
@@ -251,7 +317,10 @@ public void testMergeWithExistingManifestAfterDelete() {
long baseId = base.currentSnapshot().snapshotId();
Assert.assertEquals("Should create 1 manifest for initial write",
1, base.currentSnapshot().allManifests().size());
+ V2Assert.assertEquals("Last sequence number should be 1", 1,
readMetadata().lastSequenceNumber());
+ V1Assert.assertEquals("Table should end with last-sequence-number 0", 0,
readMetadata().lastSequenceNumber());
Review comment:
Adding the sequence number asserts between the assertion about the
number of manifests and validating a manifest makes it harder to read the test
because lines that are related are now separated.
Can you move the snapshot and table metadata sequence number assertions to
just after each commit?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
