CNTRLPLANE-2491:test/e2e: migrate serving-cert-secret-add-data test for OTE compatibility #299
Conversation
…lity Extract inline test body from e2e_test.go into shared function for dual framework compatibility (standard Go tests and OTE/Ginkgo). - Add Ginkgo wrapper in test/e2e/e2e.go - Create testServingCertSecretAddData() shared function - Refactor e2e_test.go to call shared function - Zero duplication of test logic Files changed: 2 (test/e2e/e2e.go, test/e2e/e2e_test.go)
WalkthroughRefactors the serving certificate secret mutation test by extracting inline test logic from the e2e test suite into a new dedicated helper function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing touches
Comment |
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/e2e/e2e.go:
- Around line 49-55: The Ginkgo test description is incorrect: update the
description string in the g.It call that wraps testServingCertSecretAddData to
reflect that extra data is removed (not preserved). Modify the fmt.Sprintf
message used in the g.It invocation (the test name passed to g.It) to say
something like "should remove extra data from serving cert secrets with
headless=%v" (or otherwise match the function docstring and assertions performed
by testServingCertSecretAddData, which relies on pollForSecretChangeGinkgo and
checkServiceServingCertSecretData to verify the "foo" key is removed while
TLSCertKey remains).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
test/e2e/e2e.gotest/e2e/e2e_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/e2e_test.gotest/e2e/e2e.go
🔇 Additional comments (2)
test/e2e/e2e_test.go (1)
1062-1071: LGTM!The refactoring follows the established pattern used by other migrated tests (
serving-cert-annotation,serving-cert-secret-modify-bad-tlsCert). The migration comment is consistent and the delegation to the shared function maintains test behavior.test/e2e/e2e.go (1)
315-353: LGTM!The function implementation correctly follows the pattern established by
testServingCertSecretModifyBadTLSCert. The test logic properly verifies that adding extra data triggers cleanup by the controller without regenerating the TLS certificate.
wangke19
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@wangke19: This pull request references CNTRLPLANE-2491 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gangwgr, wangke19 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/verified by CI tests |
openshift-ci-robot
added
the
verified
|
@wangke19: This PR has been marked as verified by DetailsIn response to this:
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. |
|
/test e2e-aws |
|
@wangke19: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
3 participants
Summary
Migrate the
serving-cert-secret-add-datatest to OTE (OpenShift Tests Extension) framework following the established pattern from PR #297 and PR #298.This test verifies that extra data added to a serving cert secret will NOT cause the TLS certificate to be regenerated (the controller should not remove extra data).
Changes
e2e_test.gointo shared functiontestServingCertSecretAddData()testing.TBinterface for both frameworkse2e_test.gotestTest Plan
Local verification:
CI will verify:
make update-gofmt)make verify-govet)Files Changed
test/e2e/e2e.go- Added Ginkgo wrapper and shared function (+46 lines)test/e2e/e2e_test.go- Refactored to call shared function (-32 lines)Related PRs
serving-cert-annotationmigration (merged)serving-cert-secret-modify-bad-tlsCertmigration (merged)Migration Guide
Following pattern documented in
/home/kewang/foothold-ai/OTE_GO_TEST_MIGRATION_GUIDE.mdKey principle: MOVE, DON'T COPY - Extract inline test code into shared functions that both frameworks call.