Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1402 +/- ##
==========================================
- Coverage 86.00% 85.22% -0.78%
==========================================
Files 102 102
Lines 12825 12928 +103
==========================================
- Hits 11030 11018 -12
- Misses 1307 1424 +117
+ Partials 488 486 -2
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| input []*structpb.Struct | ||
| }{ | ||
| { | ||
| name: "Test 1: Valid input with simple key-value pairs", |
There was a problem hiding this comment.
Missing cases for slices, and structs
There was a problem hiding this comment.
Should there be a test for invalid values? Empty values, blank "" values and special characters.
For valid scenario, should we include "config-sync-group" key?
There was a problem hiding this comment.
Tests for this function should be pretty generic, the exact strings we place in the tests shouldn't really matter
There was a problem hiding this comment.
This function is only to covert to a map validation will be done elsewhere
api/grpc/mpi/v1/helpers.go
Outdated
| case *structpb.Value_BoolValue: | ||
| convertedMap[key] = field.GetBoolValue() | ||
| default: | ||
| slog.Warn("unknown type for map conversion", "value", kind) |
There was a problem hiding this comment.
| slog.Warn("unknown type for map conversion", "value", kind) | |
| slog.Warn("Unknown type for map conversion", "key", key, "value", kind) |
| oc.restartCollector(ctx) | ||
| } | ||
| } else { | ||
| oc.config = agentConfig |
There was a problem hiding this comment.
Can you add an info log here saying that new headers have being added and the collector is bing restarted
Proposed changes
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTINGdocumentmake install-toolsand have attached any dependency changes to this pull requestREADME.md)