rdblue commented on PR #18853:
URL: https://github.com/apache/iceberg/pull/18853#issuecomment-6781307762
I don't think that I agree with the test plan. Looks like it is too specific
in areas and also duplicates cases. Here's what I would do:
- For every wrapper, validate the `get` method returns the expected field
data.
- Use mocks for sub-fields
- Always use `TrackedFile`, probably a mock. No need to test `DataFile`
wrappers.
- All fields should be tested for distinct cases (data file, manifest
file, delete manifest), instead of some fields for different cases (like sort
order tested only when converting from `DataFile`)
- Test that substitutions are made correctly
- `specId` is null for unpartitioned or null spec
- `mdvBuffer` and `minSequenceNumber` for `ManifestInfoWrapper`
- `firstRowId`, `dataSequenceNumber`, `fileSequenceNumber` are null for
leaf `Tracking` (ADDED status)
- `firstRowId`, `dataSequenceNumber`, `fileSequenceNumber` are non-null
for root `Tracking` (ADDED status)
- Test validations that are performed in `wrap`
- Only data and delete manifests have `ManifestInfo`
- Delete files are rejected
- Only data files can have `DeletionVector`
- `Tracking.firstRowId` must be `null` for ADDED, must not be null for
other status
- `firstRowId` passed in is non-null if and only if
`Tracking.firstRowId` is null
- If `dataSequenceNumber` is null, the snapshot ID or modified snapshot
ID (MODIFIED) must match the commit snapshot ID
- If `fileSequenceNumber` is null, the snapshot ID must match the commit
snapshot ID
I'd start there are check for any validations or substitutions that I've
missed. I also think that this will be a lot easier with a simpler structure
for the tracked file and tracking wrappers.
--
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]
