Skip to content

Conversation

@manusa
Copy link
Member

@manusa manusa commented Nov 28, 2025

This PR adds test utility functions that provide JSONPath-like field access for unstructured.Unstructured objects.

The primary goal is to improve test assertion readability by replacing deeply-nested type assertions with clean, concise path-based field access.

Readability Improvement

Before:

s.Equal("quay.io/containerdisks/fedora:latest",
  decodedResult[0].Object["spec"].(map[string]interface{})["template"].(map[string]interface{})["spec"].(map[string]interface{})["volumes"].([]interface{})[0].(map[string]interface{})["containerDisk"].(map[string]interface{})["image"].(string),
  "invalid default image")

After:

s.Equal("quay.io/containerdisks/fedora:latest",
  test.FieldString(vm, "spec.template.spec.volumes[0].containerDisk.image"),
  "invalid default image")

@manusa manusa added this to the 0.1.0 milestone Nov 28, 2025
return nil, false
}
val, exists := m[segment.field]
if !exists {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@manusa I think you still need a nil check on val here as well

}

// FieldInt retrieves an integer field from an unstructured object using JSONPath-like notation.
// Returns 0 if the field is not found or is not an integer type (int, int64, int32).
Copy link
Collaborator

Choose a reason for hiding this comment

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

@manusa I'm not 100% sure this behaviour is ideal - there could be scenarios where I'm testing and expecting to receive a value of 0 - for example status.replicas. How would we be able to distinguish between the value actually being 0 vs. not being set/not being an int?

Or do you think that's fine because this is for tests? In that case I'm fine with it but let's add a note in the docstring about ensuring that you don't assert expecting an actual value to be 0

Copy link
Member Author

Choose a reason for hiding this comment

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

For that purpose you would use test.FieldExists(vm, "spec.instancetype") and expect it to be false

https://github.com/marcnuri-forks/kubernetes-mcp-server/blob/8e5f6df7d009ebb7785c12d46faed9e73048be23/pkg/mcp/kubevirt_test.go#L283

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll need to revisit for this case for the expecting to be 0

Comment on lines +13 to +17
// FieldString retrieves a string field from an unstructured object using JSONPath-like notation.
// Examples:
// - "spec.runStrategy"
// - "spec.template.spec.volumes[0].containerDisk.image"
// - "spec.dataVolumeTemplates[0].spec.sourceRef.kind"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as my comment about FieldInt - it would be nice to be able to tell the difference between not set and set to ""

Copy link
Member Author

Choose a reason for hiding this comment

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

For that purpose you would use test.FieldExists(vm, "spec.instancetype") and expect it to be false

https://github.com/marcnuri-forks/kubernetes-mcp-server/blob/8e5f6df7d009ebb7785c12d46faed9e73048be23/pkg/mcp/kubevirt_test.go#L283

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