Split GGUF builder tests into focused modules - #705
Open
justinchuby wants to merge 1 commit into
Open
Conversation
Extract shared synthetic GGUF writers and fixtures into a private support module, partition the collected tests by responsibility, and update evidence and cross-test references without changing test behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Performance Comparison
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The updated test-infrastructure map currently omits still-present key GGUF integration test modules, making the documentation misleading.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors the GGUF builder test suite by splitting the former monolithic builder test module into multiple focused test modules and extracting shared synthetic GGUF writers/fixtures into a private, non-collected utility module. It also updates cross-test imports plus references in quantization evidence and documentation so internal “evidence → test” links remain accurate.
Changes:
- Added
src/mobius/integrations/gguf/_builder_test_utils.pyto centralize synthetic GGUF writers and shared fixtures for builder tests. - Split builder tests into focused modules (core/architectures/contracts) and updated dependent tests/import sites accordingly.
- Updated quantization evidence references and docs to point at the new test module locations.
File summaries
| File | Description |
|---|---|
| testdata/evidence/gguf_quantization_capabilities.json | Updates evidence-to-test node IDs to the new builder test module paths. |
| src/mobius/integrations/gguf/_quant_capabilities.py | Updates hard-coded test references for quantization capability evidence. |
| src/mobius/integrations/gguf/_mtp_test.py | Switches from importing the removed _builder_test to using _builder_test_utils. |
| src/mobius/integrations/gguf/_mmproj_test.py | Updates docstring reference to the former builder test module. |
| src/mobius/integrations/gguf/_dense_c01_test.py | Switches helper import to _builder_test_utils. |
| src/mobius/integrations/gguf/_builder_test_utils.py | New private helper module containing synthetic GGUF writers and shared fixtures. |
| src/mobius/integrations/gguf/_builder_contracts_test.py | New focused test module for builder preflight/cache/tensor/weight contracts. |
| src/mobius/integrations/gguf/_builder_core_test.py | New focused test module for core builder/quantization/reuse behaviors. |
| src/mobius/integrations/gguf/_builder_architectures_test.py | New focused test module for architecture-cohort builder behaviors. |
| src/mobius/integrations/gguf/_builder_test.py (deleted) | Removes the original monolithic collected builder test module after splitting. |
| docs/design/test-infrastructure-map.md | Updates documentation references for the GGUF test suite layout. |
Review details
- Files reviewed: 8/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| | **Components** | 67 | 379 | 23 test files (rotary_embedding 28M, quantized_linear 22M, audio 16M, attention 17M, vision 16M, whisper 20M, etc.) | | ||
| | **Weight Handling** | 17 | 114 | `_weight_utils_test.py` (58M), `_weight_loading_test.py` (27M), `_graph_diff_test.py` (24M) | | ||
| | **GGUF Integration** | 19 | 91 | `_builder_test.py`, `_reader_test.py` (26M), `_repacker_test.py` (30M), `_tensor_mapping_test.py` (18M) | | ||
| | **GGUF Integration** | 19 | 91 | `_builder_core_test.py`, `_builder_architectures_test.py`, `_builder_contracts_test.py`, `_reader_test.py` (26M) | |
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
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.
Summary
_builder_test_utils.pyValidation
490 passedacross all replacement modules,_dense_c01_test.py, and the affected MTP cross-import test92 passedfor affected-model discovery and GGUF evidence/docs regressions (plus 20-pass confirmation after evidence path correction)git diff --checkpassed