Skip to content

Conversation

@cblecker
Copy link
Member

Summary

Add a unit test to verify all entries in the capiResources map point to existing embedded files that parse correctly. This catches mismatches between the map and actual CRD file paths at test time rather than causing runtime panics.

Problem

PR #7305 introduced an edge case CI failure where CRD file paths were renamed but the capiResources map wasn't updated to match, causing runtime panics when trying to open non-existent embedded files.

Solution

The test iterates through all entries in capiResources and calls getCustomResourceDefinition() for each path, verifying:

  • The embedded file exists
  • The file parses correctly as YAML

Changes

  • cmd/install/assets/assets_test.go (new file): Implements TestCapiResources with:
    • Positive test case: Validates all current capiResources paths exist and parse
    • Negative test case: Verifies non-existent paths correctly panic

Testing

go test -v ./cmd/install/assets/... -run TestCapiResources

All tests pass successfully. The test will catch any future mismatches between the capiResources map and embedded CRD file paths.

Add TestCapiResources to verify all entries in the capiResources map
point to existing embedded files that parse correctly. This catches
mismatches between the map and actual CRD file paths at test time
rather than causing runtime panics.

The test includes:
- Positive test iterating all capiResources paths
- Negative test verifying non-existent paths correctly panic

Follows project conventions with "When...it should..." naming format.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

Walkthrough

Adds unit tests for the assets package in cmd/install/assets/assets_test.go. The new TestCapiResources test validates that the getCustomResourceDefinition function correctly processes existing custom resource definition paths without panic and returns non-nil values, while also verifying that non-existent paths trigger panic behavior.

Changes

Cohort / File(s) Change Summary
Assets Package Tests
cmd/install/assets/assets_test.go
Added TestCapiResources unit test that iterates over capiResources paths, asserts successful execution of getCustomResourceDefinition(CRDS, path) with non-nil results, and verifies panic behavior for non-existent paths

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot requested review from csrwng and sjenning January 28, 2026 20:03
@openshift-ci openshift-ci bot added area/cli Indicates the PR includes changes for CLI and removed do-not-merge/needs-area labels Jan 28, 2026
@csrwng csrwng changed the title test(assets): add unit test for CAPI resources file paths NO-JIRA: test(assets): add unit test for CAPI resources file paths Jan 28, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 28, 2026
@openshift-ci-robot
Copy link

@cblecker: This pull request explicitly references no jira issue.

Details

In response to this:

Summary

Add a unit test to verify all entries in the capiResources map point to existing embedded files that parse correctly. This catches mismatches between the map and actual CRD file paths at test time rather than causing runtime panics.

Problem

PR #7305 introduced an edge case CI failure where CRD file paths were renamed but the capiResources map wasn't updated to match, causing runtime panics when trying to open non-existent embedded files.

Solution

The test iterates through all entries in capiResources and calls getCustomResourceDefinition() for each path, verifying:

  • The embedded file exists
  • The file parses correctly as YAML

Changes

  • cmd/install/assets/assets_test.go (new file): Implements TestCapiResources with:
  • Positive test case: Validates all current capiResources paths exist and parse
  • Negative test case: Verifies non-existent paths correctly panic

Testing

go test -v ./cmd/install/assets/... -run TestCapiResources

All tests pass successfully. The test will catch any future mismatches between the capiResources map and embedded CRD file paths.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@csrwng
Copy link
Contributor

csrwng commented Jan 28, 2026

/verified by unit test
/approve

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jan 28, 2026
@openshift-ci-robot
Copy link

@csrwng: This PR has been marked as verified by unit test.

Details

In response to this:

/verified by unit test
/approve

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2026
@cblecker
Copy link
Member Author

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 28, 2026

@cblecker: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copy link
Contributor

@apahim apahim left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 28, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apahim, cblecker, csrwng

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit e1aecd8 into openshift:main Jan 28, 2026
19 checks passed
@cblecker cblecker deleted the test/capi-resources-validation branch January 28, 2026 22:57
@cblecker
Copy link
Member Author

/area platform/gcp

@openshift-ci openshift-ci bot added the area/platform/gcp PR/issue for GCP (GCPPlatform) platform label Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cli Indicates the PR includes changes for CLI area/platform/gcp PR/issue for GCP (GCPPlatform) platform jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants