-
Notifications
You must be signed in to change notification settings - Fork 201
[Test]Add Model URL Connectivity Tests and Improve URL Handling #560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors URL handling infrastructure for HuggingFace and GitHub model repositories by introducing static factory methods to replace string concatenation. The changes centralize URL construction logic and improve type safety.
Changes:
- Added static factory methods (
BuildRepoUrl,BuildTreeUrl,BuildResolveUrl,BuildBlobUrl,BuildApiUrl,BuildRawUrl) toHuggingFaceUrlandGitHubUrlclasses - Updated all internal usages to use new factory methods in
ModelInformationHelper,HuggingFaceApi,GithubApi,AddHFModelView, andModelDownload - Enhanced error handling with HTTP status code validation and file path validation
- Added comprehensive test coverage with 589 lines of tests across unit, API, and integration tests
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| AIDevGallery.Utils/ModelUrl.cs | Added static factory methods for URL construction to HuggingFaceUrl and GitHubUrl classes; updated existing methods to use new factories |
| AIDevGallery.Utils/ModelInformationHelper.cs | Replaced string concatenation with static factory method calls for API and resolve URLs |
| AIDevGallery/Utils/HuggingFaceApi.cs | Updated to use BuildResolveUrl factory method; added validation for modelId format and file path; enhanced error handling with HTTP status codes |
| AIDevGallery/Utils/GithubApi.cs | Updated to use BuildRawUrl factory method; added validation for file path and HTTP status code checking |
| AIDevGallery/Controls/ModelPicker/AddHFModelView.xaml.cs | Updated UI code to use factory methods for building tree, blob, and repo URLs |
| AIDevGallery/Utils/ModelDownload.cs | Added validation to check for empty file list after filtering |
| AIDevGallery.Tests/UnitTests/Utils/ModelUrlTests.cs | New comprehensive unit tests for URL parsing and construction (326 lines) |
| AIDevGallery.Tests/UnitTests/Utils/ApiTests.cs | New unit tests for API utility functions (126 lines) |
| AIDevGallery.Tests/IntegrationTests/ApiIntegrationTests.cs | New integration tests for API connectivity (101 lines) |
| AIDevGallery.Tests/IntegrationTests/ModelUrlConnectivityTests.cs | New integration tests to validate model URLs in definition files (356 lines) |
AIDevGallery.Tests/IntegrationTests/ModelUrlConnectivityTests.cs
Outdated
Show resolved
Hide resolved
AIDevGallery.Tests/IntegrationTests/ModelUrlConnectivityTests.cs
Outdated
Show resolved
Hide resolved
c6ebfb8 to
c774b63
Compare
This commit refactors the URL handling infrastructure for HuggingFace and GitHub model repositories, replacing string concatenation with type-safe static factory methods. ## Key Changes ### URL Construction Refactoring - Added static factory methods to HuggingFaceUrl and GitHubUrl classes: - BuildRepoUrl(), BuildTreeUrl(), BuildResolveUrl(), BuildBlobUrl(), BuildApiUrl() - Centralized URL format management for better maintainability - Eliminated error-prone string concatenation across codebase ### Enhanced URL Parsing - Extended HuggingFaceUrl to support 'resolve' URL format (previously only 'blob') - Improved URL validation with explicit path requirement checks - Added defensive null checks for file operations ### Improved Error Handling - Added HTTP status code validation in API calls - Enhanced error messages with detailed context (URL, status code) - Implemented empty file list validation after filtering ### Code Quality Improvements - Replaced hardcoded URL strings with named constants - Unified URL construction patterns across all API utilities - Improved code readability and type safety ### Comprehensive Test Coverage - Added 589 lines of unit tests covering all URL parsing scenarios - Created 63 test cases for both HuggingFace and GitHub URL handling - Added integration tests for API connectivity - Validated edge cases: whitespace handling, invalid formats, empty strings ## Backward Compatibility **All changes are fully backward compatible:** - Existing public APIs remain unchanged (constructors, properties, methods) - New static methods are pure additions, no breaking changes - All current code continues to work without modification - Internal refactoring does not affect external consumers ## Testing - All new static methods comprehensively tested - Verified URL parsing for various formats (tree, blob, resolve) - Tested local path mapping functionality - Validated error handling for malformed URLs This refactoring improves code maintainability, reduces bugs from string concatenation errors, and provides a more robust foundation for future URL-related features.
c774b63 to
d4c6feb
Compare
36990b3 to
eb976c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
AIDevGallery.Tests/IntegrationTests/ModelUrlConnectivityTests.cs
Outdated
Show resolved
Hide resolved
…orization - Add test categories (Integration, Network, Slow) for better test filtering - Fix HttpClient resource leak with ClassCleanup disposal - Use ClassCleanupBehavior.EndOfClass to fix MSTEST0034 warning - Add warning comment about batch test execution time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
This PR introduces comprehensive testing infrastructure to validate model download URLs and refactors URL handling code for better maintainability, robustness, and testability.
Key Highlights
New Integration Test Suite:
ModelUrlConnectivityTests.csThe centerpiece of this PR is a robust test suite that automatically validates all model URLs defined in
.model.jsonand.modelgroup.jsonfiles across the codebase. This test:Business Value: This test suite acts as an early warning system, helping developers quickly identify and fix broken model links before they impact users.
Changes
1. New Test Files
ModelUrlConnectivityTests.cs(352 lines): Integration tests that make real HTTP requests to validate model URL accessibilityModelUrlTests.cs(589 lines): Comprehensive unit tests for URL parsing, construction, and edge cases without network callsApiIntegrationTests.cs(101 lines): Integration tests for GitHub and HuggingFace API functionalityApiTests.cs(126 lines): Unit tests for API helper classes2. Enhanced
ModelUrl.csUtility ClassBefore: URLs were constructed inline with string concatenation throughout the codebase
After: Centralized URL building methods with proper validation
New static builder methods added:
HuggingFaceUrl:BuildRepoUrl(org, repo)- Base repository URLsBuildTreeUrl(org, repo, ref, path)- Directory browsing URLsBuildResolveUrl(org, repo, ref, filePath)- File download URLsBuildBlobUrl(org, repo, ref, filePath)- File viewing URLsBuildApiUrl(org, repo, ref, path)- API endpointsGitHubUrl:BuildRepoUrl(org, repo)- Base repository URLsBuildApiUrl(org, repo, ref, path)- API endpointsBuildRawUrl(org, repo, ref, filePath)- Raw file download URLsBuildBlobUrl(org, repo, ref, filePath)- File viewing URLsBenefits:
3. Refactored
ModelInformationHelper.cs4. Updated
AddHFModelView.xaml.csorg/repoformat)5. Improved API Helpers (
GithubApi.cs,HuggingFaceApi.cs)6. Enhanced
ModelDownload.csTesting Strategy
The test suite validates URLs at two levels:
Unit Tests (
ModelUrlTests.cs,ApiTests.cs):Integration Tests (
ModelUrlConnectivityTests.cs,ApiIntegrationTests.cs):Test Coverage
The connectivity tests cover:
.model.jsonfiles.modelgroup.jsonfilesFuture Benefits
Note
Integration tests make real HTTP requests and should be run separately from unit tests in CI/CD pipelines to avoid: