Skip to content

Conversation

@HeartLinked
Copy link
Contributor

@HeartLinked HeartLinked commented Aug 11, 2025

V2 Manifest Format Support

  • Add V2 Non-Partitioned Tests for ManifestListReader.

Unified Test Architecture

  • ManifestListReaderTestBase: New base class providing shared functionality for all manifest reader tests
  • Consistent Inheritance: Both V1 and V2 test classes inherit from TempFileTestBase for uniformity
  • Unified Helper Methods: Common TestManifestListReading() and TestNonPartitionedManifests() utilities

@wgtmac
Copy link
Member

wgtmac commented Aug 13, 2025

The PR title need to be changed to test: add ManifestListReaderV2Test

@HeartLinked HeartLinked changed the title test: add V2NonPartitionedTest for ManifestListReader test: add ManifestListReaderV2Test Aug 14, 2025
  for ManifestListReader
- Apply clang-format to fix code style issues
- Remove trailing whitespaces
- Fix indentation and alignment
- Ensure consistent formatting with project standards
- Rename ManifestListReaderTest to ManifestListReaderV2Test
- Rename PrepareTestManifestList to PrepareV2PartitionedTestManifestList
- Rename BasicTest to PartitionedTest
- Rename V2NonPartitionedTest to NonPartitionedTest
- Prepare structure for adding V1 tests
- Introduce unified ManifestListReaderTestBase for common functionality
- Unify V1 and V2 test class inheritance and setup patterns
- Extract test data preparation into dedicated helper methods
- Add TestNonPartitionedManifests() helper for validation
- Standardize test naming: remove version prefixes, use consistent patterns
- Improve code organization and reduce duplication
- Maintain all existing test coverage while enhancing maintainability
@HeartLinked HeartLinked force-pushed the test_manifest_list_reader_v2 branch from d8ada3e to fa0de35 Compare August 14, 2025 04:42
@HeartLinked HeartLinked changed the title test: add ManifestListReaderV2Test test:  enhance ManifestListReaderV2Test and unified test architecture Aug 14, 2025
@wgtmac
Copy link
Member

wgtmac commented Aug 14, 2025

@Fokko @zeroshade Could you help review this? Thanks!

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for working on this!

@Xuanwo Xuanwo merged commit a54f116 into apache:main Aug 15, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants