Skip to content

test: add unit tests#94

Merged
northtyphoon merged 6 commits intomainfrom
avtakkar/ut
Mar 31, 2025
Merged

test: add unit tests#94
northtyphoon merged 6 commits intomainfrom
avtakkar/ut

Conversation

@avtakkar
Copy link
Contributor

No description provided.

@avtakkar avtakkar requested a review from a team as a code owner March 28, 2025 23:02
@avtakkar avtakkar requested a review from Copilot March 28, 2025 23:03
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 PR adds unit tests for the metrics interface, ensuring the expected behavior of context manipulation functions. It also refactors the definition of ctxKey by moving it from pkg/metrics/prometheus.go to pkg/metrics/interface.go and updates related documentation comments.

  • Added unit tests for WithContext and FromContext in pkg/metrics/interface_test.go.
  • Removed a duplicate definition of ctxKey from pkg/metrics/prometheus.go.
  • Moved ctxKey to pkg/metrics/interface.go with updated comments supporting the metrics recorder context integration.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/metrics/prometheus.go Removed duplicate ctxKey declaration to centralize its usage.
pkg/metrics/interface_test.go Added unit tests for the metrics interface using WithContext and FromContext.
pkg/metrics/interface.go Added ctxKey declaration and updated WithContext comments.
Comments suppressed due to low confidence (1)

pkg/metrics/interface_test.go:21

  • Consider checking the type assertion using the ok idiom to provide a clearer error message if ctx.Value(ctxKey{}) does not contain a *promMetrics. For example, use 'm, ok := ctx.Value(ctxKey{}).(*promMetrics)' and handle the false case appropriately.
m := ctx.Value(ctxKey{}).(*promMetrics)

@codecov
Copy link

codecov bot commented Mar 28, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 62.28%. Comparing base (29d5927) to head (0e79ca2).

Files with missing lines Patch % Lines
pkg/containerd/mock.go 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (55.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
+ Coverage   61.01%   62.28%   +1.27%     
==========================================
  Files          38       38              
  Lines        2352     2352              
==========================================
+ Hits         1435     1465      +30     
+ Misses        917      887      -30     
Files with missing lines Coverage Δ
pkg/metrics/interface.go 77.77% <ø> (+77.77%) ⬆️
pkg/metrics/prometheus.go 100.00% <ø> (ø)
pkg/containerd/mock.go 54.05% <0.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@avtakkar avtakkar requested a review from Copilot March 29, 2025 00:07
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 PR adds unit tests for the metrics interface and updates related code to support these tests. Key changes include:

  • Removing the duplicate ctxKey struct definition from pkg/metrics/prometheus.go.
  • Adding new unit tests for metrics functions in pkg/metrics/interface_test.go.
  • Modifying error handling in pkg/containerd/mock.go to return an error when no content is found.
  • Updating ignore patterns in .github/.codecov.yml to exclude mock files.

Reviewed Changes

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

Show a summary per file
File Description
pkg/metrics/prometheus.go Removed extraneous duplicate definition of ctxKey
pkg/metrics/interface_test.go Added tests to validate the metrics interface behavior
pkg/metrics/interface.go Introduced ctxKey type to be shared across metrics context functions
pkg/handlers/v2/handler_test.go Added tests covering various handler responses under different scenarios
pkg/containerd/mock.go Updated error handling in the Bytes function to return an error instead of nil
.github/.codecov.yml Updated ignore patterns to exclude mock files

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Aviral Takkar <39969667+avtakkar@users.noreply.github.com>
@avtakkar avtakkar changed the title test: add unit tests for metrics interface test: add unit tests Mar 29, 2025
Copy link
Member

@northtyphoon northtyphoon left a comment

Choose a reason for hiding this comment

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

LGTM

@northtyphoon northtyphoon merged commit 1747e4a into main Mar 31, 2025
6 checks passed
@avtakkar avtakkar deleted the avtakkar/ut branch March 31, 2025 16:17
avtakkar added a commit to avtakkar/peerd that referenced this pull request Apr 23, 2025
* test: add unit tests for metrics interface

* fix test

* add more tests

* add another test

* ignore mock files from coverage

* Update pkg/containerd/mock.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Aviral Takkar <39969667+avtakkar@users.noreply.github.com>

---------

Signed-off-by: Aviral Takkar <39969667+avtakkar@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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