Skip to content

Conversation

@saanikaguptamicrosoft
Copy link
Collaborator

No description provided.

Copy link
Contributor

Copilot AI left a 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 pull request adds comprehensive unit test coverage for the azure.ai.finetune extension. The tests cover models, utilities, services, providers, and command functionality with table-driven patterns and thorough edge case testing.

Changes:

  • Adds 16 new test files covering all major components of the azure.ai.finetune extension
  • Adds 13 testdata YAML files for parser validation testing
  • Fixes spacing alignment for a constant in environment.go (formatting only)

Reviewed changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/models/views_test.go Tests for view model transformations and formatting helpers
pkg/models/requests_test.go Tests for request structure validation
pkg/models/finetune_test.go Tests for fine-tuning job models, validation, and JSON marshaling
pkg/models/errors_test.go Tests for error handling and error code uniqueness
pkg/models/deployment_test.go Tests for deployment models and configurations
internal/utils/time_test.go Tests for time formatting and duration calculations
internal/utils/status_test.go Tests for job status symbols and terminal status detection
internal/utils/retry_test.go Tests for retry logic with various failure scenarios
internal/utils/parser_test.go Tests for YAML config parsing with valid and invalid inputs
internal/utils/output_test.go Tests for output formatting (table, JSON, YAML)
internal/utils/environment_test.go Tests for environment constant validation
internal/utils/common_test.go Tests for file path utilities
internal/services/finetune_service_test.go Tests for fine-tuning service with mock providers
internal/providers/azure/provider_test.go Tests for Azure provider validation
internal/cmd/init_test.go Tests for initialization command and resource ID parsing
internal/utils/environment.go Formatting fix: aligns EnvFinetuningTokenScope constant spacing
internal/utils/testdata/*.yaml Test fixture files for parser validation tests

Comment on lines +449 to +455
func generateMetadata(count int) map[string]string {
metadata := make(map[string]string)
for i := 0; i < count; i++ {
metadata[string(rune('a'+i))] = "value"
}
return metadata
}
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The generateMetadata function will fail when count exceeds 26 (number of letters in the alphabet). When generating metadata with count > 26, the expression 'a' + i will go beyond 'z', creating invalid single-character keys. This could cause tests to pass with incorrect data when testing edge cases like the maximum of 16 metadata entries.

Consider using a more robust key generation approach such as using fmt.Sprintf to generate keys like "key0", "key1", etc.

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +66
- pwsh: |
Write-Host "Running unit tests..."
go test ./... -v -count=1
Copy link
Contributor

Choose a reason for hiding this comment

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

@danieljurek for extensions do we need a separate ci-test.ps1 for unit tests or is this fine?

Copy link
Member

Choose a reason for hiding this comment

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

Correct. The intent here was to run something like ci-test.ps1 in the same way that we run ci-build.ps1 but that was not standardized. I'll open a PR to get this into the right shape.

Copy link
Member

Choose a reason for hiding this comment

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

PR to add basic unit testing support: #6560

Copy link
Member

Choose a reason for hiding this comment

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

Issue to further scope and think about this: #6561

Copy link
Member

Choose a reason for hiding this comment

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

Merged. This PR should be unblocked now. Rebase on main and put the test code in ci-test.ps1 (you shouldn't have to edit anything in eng/).

Write-Host "Running unit tests..."
go test ./... -v -count=1

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.

3 participants