Skip to content

Conversation

@n3rdc4ptn
Copy link
Member

This pull request introduces several enhancements and fixes across the codebase, focusing on task management, API updates, logging improvements, and test coverage. The most significant changes include the addition of new tasks in Taskfile.yaml, updates to the MCP usage structure, and improvements to logging and test infrastructure.

Task Management and Configuration Updates:

  • Added new tasks in Taskfile.yaml for generating code and fetching external APIs, including a download-crds task to fetch CRD files from external repositories (Taskfile.yaml) [1] [2].

MCP Usage and API Changes:

  • Updated MCPUsageSpec to make the daily_usage field optional by adding the omitempty tag (api/usage/v1/mcpusage_types.go).
  • Removed daily_usage from the required fields in the CRD manifest (api/crds/manifests/usage.openmcp.cloud_mcpusages.yaml).

Logging Improvements:

  • Replaced the custom logging utility with logr for consistency and better integration with the controller-runtime library (internal/controller/managedcontrolplane_controller.go) [1] [2] [3].

Test Enhancements:

  • Added comprehensive unit tests for the ChargingTarget resolver and MCP usage reconciliation logic (internal/helper/chargingtarget_test.go, internal/controller/managedcontrolplane_controller_test.go) [1] [2].
  • Introduced a new test suite for helper functions with improved test environment setup (internal/helper/suite_test.go).

Dependency and Code Cleanup:

  • Added github.com/go-logr/logr as a direct dependency in go.mod and removed its indirect reference for clarity (go.mod) [1] [2].
  • Simplified error handling in the MergeDailyUsages function by removing redundant comments (internal/usage/helper.go).

@n3rdc4ptn n3rdc4ptn requested a review from Copilot July 24, 2025 11:27
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 introduces comprehensive test coverage for the usage tracking functionality and logging improvements. The changes include transitioning from custom logging to the standard logr library, adding extensive unit tests for helper functions and controllers, and enhancing the test infrastructure setup.

Key changes include:

  • Migration from custom logging utility to logr for better integration with controller-runtime
  • Addition of comprehensive test suites for usage tracking, charging target resolution, and controller reconciliation
  • Enhancement of test infrastructure with proper CRD loading and environment setup

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/usage/tracking.go Replaced custom logging with logr and updated logging calls
internal/usage/tracking_test.go Added comprehensive tests for usage tracking functionality
internal/usage/suite_test.go Enhanced test suite setup with proper CRD loading
internal/usage/helper_test.go Added tests for helper functions including usage merging
internal/helper/chargingtarget_test.go Added tests for charging target resolution logic
internal/controller/managedcontrolplane_controller_test.go Added controller reconciliation tests
internal/controller/suite_test.go Enhanced controller test suite with manager setup
go.mod Made logr a direct dependency
Comments suppressed due to low confidence (1)

internal/usage/tracking.go:55

  • The logr.FromContext function does not return an error. It should be called as log := logr.FromContext(ctx) without error handling.
		}

@n3rdc4ptn n3rdc4ptn merged commit 8d0259b into main Jul 24, 2025
6 checks passed
@n3rdc4ptn n3rdc4ptn deleted the add-testing branch July 24, 2025 11:37
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.

2 participants