feat(test): add end-to-end validation for architecture_config integration#199
feat(test): add end-to-end validation for architecture_config integration#199rishi-jat wants to merge 2 commits intomodelpack:mainfrom
Conversation
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial end-to-end integration test to ensure the robust handling of Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces schema/architecture_integration_test.go, which includes a utility function convertHFToArchConfig for converting HuggingFace configurations to an internal architecture format, along with several integration tests. These tests cover valid conversions, missing fields, invalid model structures, and invalid field values, and also validate the full pipeline including schema validation. Feedback suggests improving the archConfig map verification in TestArchitectureIntegrationValidConversion by using reflect.DeepEqual for robustness and conciseness. Additionally, it's recommended to explicitly handle the success case in the validation logic of TestArchitectureIntegrationValidConversion to enhance clarity for its dual-purpose nature.
There was a problem hiding this comment.
Pull request overview
Adds a new Go integration test to exercise the end-to-end pipeline for deriving an architecture_config-like object from a HuggingFace-style config and embedding it into a full Model JSON for schema validation (with current expected failure behavior until the schema accepts the new field).
Changes:
- Introduces
schema/architecture_integration_test.gowith a simulated HF→architecture_configconversion helper. - Adds an end-to-end validation test that currently expects schema rejection due to
architecture_configbeing an unknown field. - Adds negative-path tests for missing HF fields, invalid model structure, and invalid numeric values.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
|
/cc @aftersnow |
Summary
Adds an integration test that validates the full pipeline for
architecture_configwithin the ModelPack model specification.This test ensures that a HuggingFace-style configuration can be converted into
architecture_config, embedded into a Model, and passed through schema validation.Scope
architecture_configschema.ValidatorMediaTypeModelConfigTest Coverage
Notes
architecture_configdepends on the schema changes introduced in the corresponding schema PR.architecture_configfield.Non-goals
This PR establishes a baseline integration check for future work on architecture specification, tooling, and validation.