Add tests for openshift-service-ca whiteout etc#32
Conversation
There is service CA configmap cleanup already. Reported issues most likely didn't use this OpenShift plugin, that needs to be added upstream manually with `$ crane plugin-manager add OpenShiftPlugin`. Adding just a test to cover this case. Related to migtools#27 and migtools#23 Signed-off-by: Marek Aufart <maufart@redhat.com>
📝 WalkthroughWalkthroughA single table-driven test ChangesConfigMap Filtering Test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openshift/plugin_test.go (1)
176-256: 💤 Low valueConsider adding test coverage for invalid flag values and response fields.
The test could be enhanced with:
- A test case for an invalid
StripDefaultCABundleFlagvalue (e.g.,"invalid") to verify error handling- Assertions for
response.Version(should bestring(transform.V1)) andresponse.Patches(should be empty for ConfigMap whiteout cases)These additions would provide more complete coverage but are not critical since the current test validates the core filtering behavior.
Example test case for invalid flag value
+ { + name: "invalid strip-default-cabundle value should error", + configMapName: "openshift-service-ca.crt", + extras: map[string]string{StripDefaultCABundleFlag: "invalid"}, + expectWhiteOut: false, + description: "Invalid boolean value should return error", + },And update the error handling to distinguish between expected errors:
response, err := plugin.Run(request) - if err != nil { + if err != nil && !strings.Contains(tt.name, "should error") { t.Fatalf("unexpected error: %v", err) } + if err == nil && strings.Contains(tt.name, "should error") { + t.Fatalf("expected error but got none") + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openshift/plugin_test.go` around lines 176 - 256, Add test coverage for invalid flag values and response field validation in the TestConfigMapFiltering function. First, add a new test case to the tests slice that covers an invalid value for the StripDefaultCABundleFlag (such as "invalid") to ensure error handling is working correctly. Second, enhance the assertion logic inside the test loop to validate not only the IsWhiteOut field but also check that response.Version equals string(transform.V1) and that response.Patches is empty for ConfigMap whiteout scenarios. This will provide more comprehensive coverage of the plugin's response contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openshift/plugin_test.go`:
- Around line 176-256: Add test coverage for invalid flag values and response
field validation in the TestConfigMapFiltering function. First, add a new test
case to the tests slice that covers an invalid value for the
StripDefaultCABundleFlag (such as "invalid") to ensure error handling is working
correctly. Second, enhance the assertion logic inside the test loop to validate
not only the IsWhiteOut field but also check that response.Version equals
string(transform.V1) and that response.Patches is empty for ConfigMap whiteout
scenarios. This will provide more comprehensive coverage of the plugin's
response contract.
Signed-off-by: Marek Aufart <maufart@redhat.com>
There is service CA configmap cleanup already. Reported issues most likely didn't use this OpenShift plugin, that needs to be added upstream manually with
$ crane plugin-manager add OpenShiftPlugin.Adding just a test to cover this case.
Related to #27 and #23
Summary by CodeRabbit