CM-875: Adds API test framework, testcases and the required vendor code#373
CM-875: Adds API test framework, testcases and the required vendor code#373bharath-b-rh wants to merge 8 commits intoopenshift:masterfrom
Conversation
Signed-off-by: Bharath B <bhb@redhat.com>
Signed-off-by: Bharath B <bhb@redhat.com>
Signed-off-by: Bharath B <bhb@redhat.com>
Signed-off-by: Bharath B <bhb@redhat.com>
Signed-off-by: Bharath B <bhb@redhat.com>
Signed-off-by: Bharath B <bhb@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a Ginkgo-based API integration test framework, new YAML-driven CRD test suites for CertManager, IstioCSR, and TrustManager, Makefile and CI/tooling updates to install/run Ginkgo and envtest, and upgrades multiple Go module dependencies (notably Kubernetes-related modules to v0.34.4 / v1.34.4). Changes
Sequence DiagramsequenceDiagram
actor Developer/CI
participant Makefile
participant Script as hack/test-apis.sh
participant Ginkgo
participant Suite as test/apis/suite_test.go
participant Generator as test/apis/generator.go
participant Envtest
participant APIserver
participant CRDfiles as api/.../*.testsuite.yaml
Developer/CI->>Makefile: make test-apis
Makefile->>Script: invoke hack/test-apis.sh (ensures GINKGO)
Script->>Ginkgo: run ginkgo against test/apis
Ginkgo->>Suite: start TestAPIs
Suite->>Envtest: start envtest control plane
Envtest->>APIserver: provide API server
Suite->>Generator: LoadTestSuiteSpecs()
Generator->>CRDfiles: scan and load .testsuite.yaml
Generator-->>Suite: generated Ginkgo test tables
Suite->>APIserver: install CRD(s)
Suite->>APIserver: create/update resource per case
APIserver-->>Suite: return observed state/result
Suite->>APIserver: cleanup resources and uninstall CRD
Suite->>Envtest: stop envtest
Ginkgo-->>Script: report exit status
Script-->>Developer/CI: exit with status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
hack/test-apis.sh (1)
18-20: Use[[ ... ]]and quote variables for robustness.The comparison at line 18 should use
[[ ... == ... ]]for string comparison and quoted variables to handle edge cases properly.🔧 Suggested improvement
-if [ $HOME == "/" ]; then +if [[ "${HOME}" == "/" ]]; then export HOME=/tmp/kubebuilder-testing fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/test-apis.sh` around lines 18 - 20, The shell conditional in hack/test-apis.sh uses [ $HOME == "/" ] which is brittle; update the if statement to use the bash-style test [[ ... ]] and quote the variable, i.e. change the check around HOME to use [[ "$HOME" == "/" ]] and leave the export HOME=/tmp/kubebuilder-testing unchanged so it correctly handles empty or whitespace-containing HOME values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@api/operator/v1alpha1/tests/certmanagers.operator.openshift.io/certmanager.testsuite.yaml`:
- Around line 743-756: The expectedError in the test named "Should not allow
changing defaultNetworkPolicy from true to empty" is incorrect: it asserts a
transition message "'true' to 'false'" while the update payload changes "true"
to "" (empty string). Update the expectedError for this test (resourceName:
cluster) to reflect the actual transition from 'true' to empty (e.g.,
"spec.defaultNetworkPolicy: Invalid value: \"string\": defaultNetworkPolicy
cannot be changed from 'true' to '' once set" or equivalent wording used by
validation) so the assertion matches the update payload.
In `@go.mod`:
- Around line 128-134: The go.mod entry for k8s.io/kube-aggregator is
inconsistent (v0.34.1) with other Kubernetes deps at v0.34.4; update the
k8s.io/kube-aggregator module version to v0.34.4 to match the rest (or, if there
is a deliberate reason to keep v0.34.1, add a comment/replace directive
documenting that constraint), and then run `go get
k8s.io/kube-aggregator@v0.34.4` (or the appropriate go command) to refresh
go.sum so all Kubernetes dependencies stay aligned.
In `@hack/govulncheck.sh`:
- Around line 24-26: Remove or update the stale comment that mentions "Below
vulnerabilities are in the go packages" which no longer matches the current
KNOWN_VULNS_PATTERN variable; specifically edit the comment block above the
KNOWN_VULNS_PATTERN declaration to either remove the Go-package wording or
replace it with a correct description indicating the pattern contains
Kubernetes-related vulnerabilities (referencing the KNOWN_VULNS_PATTERN constant
and the surrounding comment area), so the comment accurately reflects the
contents.
In `@hack/test-apis.sh`:
- Around line 27-32: The script currently has set -o errexit enabled so the
Ginkgo invocation (the ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS}
${REPO_ROOT}/test/apis call) will cause the script to exit immediately on
failure and never assign TEST_RESULT; to fix, temporarily disable errexit (e.g.,
set +e) before invoking the Ginkgo command, run the command, capture its exit
status into TEST_RESULT (TEST_RESULT=$?), and then re-enable errexit (set -e or
set -o errexit) afterward so subsequent behavior remains unchanged; update the
block around the GINKGO invocation to implement this change referencing the
GINKGO invocation and TEST_RESULT variable.
In `@Makefile`:
- Around line 250-252: The test-unit target's package exclusion uses an
incorrect regex character class in the grep -vE call, so packages like test/e2e,
test/apis and test/utils are not properly excluded; update the grep pattern in
the go test command used by the test-unit target to use alternation (grouping
with |) for the three test subpackages (e2e, apis, utils) instead of a character
class, so the filter correctly excludes those directories when running go test.
In `@test/apis/generator.go`:
- Around line 131-132: The test currently hardcodes namespace filters
(client.InNamespace("default") and SetNamespace("default")) which will fail for
cluster-scoped CRDs (certmanagers, trustmanagers); update the cleanup/create
logic in test/apis/generator.go to check the CRD scope before applying namespace
constraints: inspect the target GVK or CRD metadata for scope and only call
SetNamespace("default") or pass client.InNamespace("default") to
k8sClient.DeleteAllOf when the resource is namespace-scoped (e.g., istiocsrs);
for cluster-scoped resources call DeleteAllOf/Create/Delete without namespace
filters or run them through a separate cluster-scoped path so certmanagers and
trustmanagers aren’t given a namespace.
- Around line 562-577: The loop over generatedCRDs currently stops after the
first matching CRD due to the stray break; remove that break so the loop
continues scanning all files and appends every match to crdFilesToCheck (using
the existing append). Keep using relativePathForCRDs, loadCRDFromFile,
generatedCRDs and crdName as-is; optionally ensure uniqueness if duplicates are
a concern, but the primary fix is to delete the break so all matching CRD
filenames are collected.
---
Nitpick comments:
In `@hack/test-apis.sh`:
- Around line 18-20: The shell conditional in hack/test-apis.sh uses [ $HOME ==
"/" ] which is brittle; update the if statement to use the bash-style test [[
... ]] and quote the variable, i.e. change the check around HOME to use [[
"$HOME" == "/" ]] and leave the export HOME=/tmp/kubebuilder-testing unchanged
so it correctly handles empty or whitespace-containing HOME values.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (286)
go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumtest/go.sumis excluded by!**/*.sumtools/go.sumis excluded by!**/*.sumvendor/github.com/charmbracelet/colorprofile/.golangci-soft.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/colorprofile/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/colorprofile/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/colorprofile/env.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/colorprofile/profile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/colorprofile/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/color.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/cursor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/finalterm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/graphics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/mode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/modes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/sgr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/style.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/title.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/truncate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/ansi/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/cellbuf/cell.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/charmbracelet/x/cellbuf/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/ghodss/yaml/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/ghodss/yaml/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/ghodss/yaml/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/ghodss/yaml/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/ghodss/yaml/fields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/ghodss/yaml/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/cmd_utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/convert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/convert_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/sizeof.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/type_constraints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/convert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/convert_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/path.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/initialism_index.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/concat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/loading.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/BENCHMARK.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/initialism_index.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/name_lexem.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/name_mangler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/pools.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/split.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/string_bytes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/name_lexem.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils/net.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/string_bytes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/collection_formats.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/env.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/folding.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/templates/authoring.tmplis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/cel/validator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/common/types/pb/type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/cel-go/ext/native.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/raw.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/unknown_fields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/maxbrunsfeld/counterfeiter/v6/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/maxbrunsfeld/counterfeiter/v6/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/maxbrunsfeld/counterfeiter/v6/arguments/parser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/maxbrunsfeld/counterfeiter/v6/generator/fake.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/maxbrunsfeld/counterfeiter/v6/generator/interface_template.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/maxbrunsfeld/counterfeiter/v6/generator/loader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/sergi/go-diff/diffmatchpatch/diff.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/sergi/go-diff/diffmatchpatch/index.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/sergi/go-diff/diffmatchpatch/stringutil.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/alias.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/basic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/cast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/caste.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/indirect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/internal/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/internal/timeformattype_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/number.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/slice.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/timeformattype_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cast/zz_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/vmware-archive/yaml-patch/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/container.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/operation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/pathfinder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/vmware-archive/yaml-patch/placeholder_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/internal/telemetry/id.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/internal/telemetry/number.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/internal/telemetry/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/internal/telemetry/status.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/internal/telemetry/traces.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/internal/telemetry/value.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/auto/sdk/tracer.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/client.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/handler.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv/env.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv/gen.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv/httpconv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv/util.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil/gen.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil/netconv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/transport.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/.codespellignoreis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/.lycheeignoreis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/CODEOWNERSis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/Makefileis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/SECURITY-INSIGHTS.ymlis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/VERSIONING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/filter.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/hash.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/internal/attribute.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/internal/xxhash/xxhash.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/iterator.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/key.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/kv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/set.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/type_string.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/value.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/baggage/baggage.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/codes/codes.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/dependencies.Dockerfileis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/global/instruments.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/global/internal_logging.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/global/meter.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/global/trace.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/asyncfloat64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/asyncint64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/meter.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/noop/noop.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/syncfloat64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/syncint64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/propagation/baggage.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/propagation/propagation.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/propagation/trace_context.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/x.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_bsd.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_linux.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_windows.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_release_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/id_generator.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/env/env.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/simple_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/snapshot.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span_limits.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/tracer.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/internal/http.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/event.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/http.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/resource.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.20.0/trace.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/exception.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/schema.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.34.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.34.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/error_type.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/exception.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/httpconv/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/schema.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/error_type.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/exception.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/otelconv/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/schema.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/auto.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/hex.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/attr.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/id.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (14)
Makefileapi/operator/v1alpha1/tests/certmanagers.operator.openshift.io/certmanager.testsuite.yamlapi/operator/v1alpha1/tests/istiocsrs.operator.openshift.io/istiocsr.testsuite.yamlapi/operator/v1alpha1/tests/trustmanagers.operator.openshift.io/trustmanager.testsuite.yamlgo.modhack/govulncheck.shhack/test-apis.shtest/apis/README.mdtest/apis/generator.gotest/apis/suite_test.gotest/apis/vars.gotest/go.modtools/go.modtools/tools.go
api/operator/v1alpha1/tests/certmanagers.operator.openshift.io/certmanager.testsuite.yaml
Show resolved
Hide resolved
| echo ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis | ||
| ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis | ||
| # Capture the test result to exit on error. | ||
| TEST_RESULT=$? | ||
|
|
||
| # Ensure we exit based on the test result |
There was a problem hiding this comment.
errexit prevents capturing test result on failure.
With set -o errexit at line 5, if the Ginkgo command at line 28 fails, the script exits immediately without reaching line 30 to capture TEST_RESULT. The result capture logic is effectively dead code on test failure.
🐛 Proposed fix to correctly capture test results
# Print the command we are going to run as Make would.
echo ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis
-${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis
-# Capture the test result to exit on error.
-TEST_RESULT=$?
-
-# Ensure we exit based on the test result
-exit ${TEST_RESULT}
+${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis || TEST_RESULT=$?
+exit ${TEST_RESULT:-0}Alternatively, if you want to keep the current structure:
+set +o errexit
# Print the command we are going to run as Make would.
echo ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis
${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis
# Capture the test result to exit on error.
TEST_RESULT=$?
+set -o errexit
# Ensure we exit based on the test result
exit ${TEST_RESULT}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis | |
| ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis | |
| # Capture the test result to exit on error. | |
| TEST_RESULT=$? | |
| # Ensure we exit based on the test result | |
| echo ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis | |
| ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis || TEST_RESULT=$? | |
| exit ${TEST_RESULT:-0} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/test-apis.sh` around lines 27 - 32, The script currently has set -o
errexit enabled so the Ginkgo invocation (the ${GINKGO} ${GINKGO_ARGS}
${GINKGO_EXTRA_ARGS} ${REPO_ROOT}/test/apis call) will cause the script to exit
immediately on failure and never assign TEST_RESULT; to fix, temporarily disable
errexit (e.g., set +e) before invoking the Ginkgo command, run the command,
capture its exit status into TEST_RESULT (TEST_RESULT=$?), and then re-enable
errexit (set -e or set -o errexit) afterward so subsequent behavior remains
unchanged; update the block around the GINKGO invocation to implement this
change referencing the GINKGO invocation and TEST_RESULT variable.
Signed-off-by: Bharath B <bhb@redhat.com>
|
@bharath-b-rh: This pull request references CM-875 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 story to target either version "4.22." or "openshift-4.22.", but it targets "cert-manager-1.19" instead. 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. |
| github.com/openshift/client-go v0.0.0-20251205093018-96a6cbc1420c | ||
| github.com/stretchr/testify v1.11.1 | ||
| github.com/tidwall/gjson v1.18.0 | ||
| github.com/vmware-archive/yaml-patch v0.0.11 |
There was a problem hiding this comment.
is there any discussion in openshift/api to replace this archived dependency?
| set -o nounset | ||
| set -o pipefail | ||
| set -o errexit |
There was a problem hiding this comment.
we are sourcing lib/init.sh instead of explicit settings right?
There was a problem hiding this comment.
Yeah, we could use that as well. Maybe we can refactor other scripts as well.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bharath-b-rh, mytreya-rh 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 |
Signed-off-by: Bharath B <bhb@redhat.com>
|
New changes are detected. LGTM label has been removed. |
|
@bharath-b-rh: This pull request references CM-875 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 story to target either version "4.22." or "openshift-4.22.", but it targets "cert-manager-1.19" instead. 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. |
|
@bharath-b-rh: 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. |
The PR has following changes:
Note: All the testcases were generated using cursor.
Summary by CodeRabbit
New Features
Chores