tests : add comprehensive GGUF format test coverage #20
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=80&v=4){kind=small}
- Extended test_handcrafted_file() with 3 new corruption scenarios: * HEADER_ENDIANNESS_MISMATCH: Test byte order detection * HEADER_N_TENSORS_MAX: Test SIZE_MAX boundary for n_tensors * HEADER_N_KV_MAX: Test SIZE_MAX boundary for n_kv - Added test_version_compatibility() for version format validation: * Validates reading GGUF version 3 files * Returns npass/ntest pair following existing patterns - Added test_large_file_handling() for stress testing: * Tests with 1000 KV pairs to validate large context handling * Tests large_kv_count and large_kv_roundtrip scenarios * Ensures scalability for realistic large files - Updated main() to call new test functions: * test_version_compatibility() runs after test_handcrafted_file() * test_large_file_handling() runs for each backend device This improves test coverage for GGUF file format handling, including edge cases, boundary conditions, and version compatibility scenarios. All 74 test checks pass (up from 67 baseline checks). Co-Authored-By: Stephen Cornwell <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Addresses editorconfig CI failure by removing trailing whitespace from 16 lines in the new test functions (test_version_compatibility and test_large_file_handling). All 74 tests still pass locally after this formatting fix. Co-Authored-By: Stephen Cornwell <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make sure to read the contributing guidelines before submitting a PR
Summary
This PR adds comprehensive test coverage to the GGUF format testing suite in
tests/test-gguf.cpp. The changes focus on testing edge cases, boundary conditions, and version compatibility scenarios to improve robustness of GGUF file format handling.Changes
Extended
test_handcrafted_file()with 3 new corruption scenarios:Added
test_version_compatibility()function:GGUF_VERSIONAdded
test_large_file_handling()function:large_kv_count: Verifies correct KV count after bulk insertionlarge_kv_roundtrip: Validates serialization/deserialization with large KV countUpdated
main()to orchestrate new tests:test_version_compatibility()runs once aftertest_handcrafted_file()test_large_file_handling()runs for each backend deviceTest Results
All tests pass: 74/74 test checks (up from 67 baseline checks)
Review Checklist
Critical areas for review:
{version >> 24, version >> 16, version >> 8, version}actually triggers endianness detection on little-endian systemstest_version_compatibility()should test multiple GGUF versions (currently only tests v3) or be renamed to reflect its actual scopetest_large_file_handling()should also test large tensor counts in addition to large KV countsSession Info