fix: resolve Memory Summary showing no data in Hardware Information (…#821
fix: resolve Memory Summary showing no data in Hardware Information (…#821amarnath-ac wants to merge 2 commits intomainfrom
Conversation
…816) Enhanced parseCIMResponse to handle typed slices using reflection. Added test for parseCIMResponse function achieving 100% coverage. Signed-off-by: C, Amarnath <amarnath.c@intel.com> Signed-off-by: S, Devipriya <devipriya.s@intel.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #821 +/- ##
=======================================
Coverage 39.76% 39.76%
=======================================
Files 114 114
Lines 10768 10768
=======================================
Hits 4282 4282
Misses 6087 6087
Partials 399 399 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@amarnath-ac Since we know the type of data which is []physical.PhysicalMemory, it's not necessary to use reflect. Reflect is generally used when the type is truly unknown at compile time. Instead, convert the typed slice to []interface{} at the source (in createMapInterfaceForHWInfo), which is consistent with how the other CIM types are handled. This way parseCIMResponse doesn't need to change at all. internal\usecase\devices\wsman\message.go: line number 534
Add the following function to the same file I have tested this fix on AMT 19 and Memory Summary loads correctly. Please verify on your end as well. |
There was a problem hiding this comment.
Pull request overview
Fixes the Hardware Information “Memory Summary shows no data” regression by making parseCIMResponse correctly handle responses values that come back as typed slices (e.g., []physical.PhysicalMemory) rather than only []interface{}.
Changes:
- Update
parseCIMResponseto convert typed slices ininfo["responses"]into[]interface{}via reflection. - Add a focused private-unit test suite for
parseCIMResponse, covering nil/invalid inputs and multipleresponses/statusshapes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/usecase/devices/info.go |
Converts typed-slice responses values to []interface{} so memory items populate correctly. |
internal/usecase/devices/info_private_test.go |
Adds comprehensive unit tests for parseCIMResponse covering typed slice handling and edge cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Thanks for feedback, will update as suggested. |
Convert PhysicalMemory slice to interface slice at source.
@madhavilosetty-intel , Updated code |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // interfaceSlice converts []physical.PhysicalMemory to []interface{} for consistent handling. | ||
| func interfaceSlice(items []physical.PhysicalMemory) []interface{} { | ||
| if items == nil { | ||
| return []interface{}{} | ||
| } | ||
|
|
||
| result := make([]interface{}, len(items)) | ||
|
|
||
| for i := range items { | ||
| result[i] = items[i] | ||
| } | ||
|
|
||
| return result |
There was a problem hiding this comment.
interfaceSlice is named as if it were a generic helper, but it only accepts []physical.PhysicalMemory. Consider renaming it to reflect the specific type/purpose (or making it generic if that’s the intent) to avoid confusion for future callers.
| func TestInterfaceSlice(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| input []physical.PhysicalMemory | ||
| expected []interface{} | ||
| }{ | ||
| { | ||
| name: "empty slice", | ||
| input: []physical.PhysicalMemory{}, | ||
| expected: []interface{}{}, | ||
| }, | ||
| { | ||
| name: "single item", | ||
| input: []physical.PhysicalMemory{ | ||
| { | ||
| Capacity: 17179869184, // 16 GB | ||
| Manufacturer: "Kingston", | ||
| PartNumber: "9905700-101.A00G", | ||
| }, | ||
| }, | ||
| expected: []interface{}{ | ||
| physical.PhysicalMemory{ | ||
| Capacity: 17179869184, | ||
| Manufacturer: "Kingston", | ||
| PartNumber: "9905700-101.A00G", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "multiple items", | ||
| input: []physical.PhysicalMemory{ | ||
| { | ||
| Capacity: 8589934592, // 8 GB | ||
| Manufacturer: "Samsung", | ||
| PartNumber: "M471A1K43CB1-CRC", | ||
| }, | ||
| { | ||
| Capacity: 8589934592, // 8 GB | ||
| Manufacturer: "Samsung", | ||
| PartNumber: "M471A1K43CB1-CRC", | ||
| }, | ||
| }, | ||
| expected: []interface{}{ | ||
| physical.PhysicalMemory{ | ||
| Capacity: 8589934592, | ||
| Manufacturer: "Samsung", | ||
| PartNumber: "M471A1K43CB1-CRC", | ||
| }, | ||
| physical.PhysicalMemory{ | ||
| Capacity: 8589934592, | ||
| Manufacturer: "Samsung", | ||
| PartNumber: "M471A1K43CB1-CRC", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "nil slice", | ||
| input: nil, | ||
| expected: []interface{}{}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range tests { | ||
| tc := tc | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| result := interfaceSlice(tc.input) | ||
| require.Equal(t, tc.expected, result) |
There was a problem hiding this comment.
PR description mentions updating/testing parseCIMResponse (and using reflection for typed slices), but this change instead introduces interfaceSlice in wsman/message.go and tests that helper. Please update the PR description (or include the missing parseCIMResponse changes/tests) so the intent and scope match what’s actually being merged.
…#816)
Enhanced parseCIMResponse to handle typed slices using reflection.
Added test for parseCIMResponse function achieving 100% coverage.