Skip to content

backend: helm repository error handling and regression test coverage#5149

Merged
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Athang69:backend/helm-repo-error-handling-tests
Apr 16, 2026
Merged

backend: helm repository error handling and regression test coverage#5149
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Athang69:backend/helm-repo-error-handling-tests

Conversation

@Athang69
Copy link
Copy Markdown
Contributor

Summary

This PR fixes Helm repository/chart handler error behavior by returning early on chart-list failures, mapping repository-not-found deletes to 404, improving repository lock error handling, and adding regression tests for these paths.

Related Issue

Fixes #ISSUE_NUMBER

Changes

  • Added/Updated backend Helm handler logic for safer error handling and HTTP status mapping
  • Fixed a flow issue where success payload logic could continue after an error
  • Added regression tests for:
    • chart list error path (no success payload on failure)
    • repository delete not-found behavior (expects 404)
    • JSON content-type behavior for list endpoints
    • repository internal behaviors via internal test coverage
  • Refactored response handling to set JSON Content-Type before writing status for JSON responses
  • Increased backend Helm package test coverage from 21.4% to 22.6% (+1.2 percentage points)

Steps to Test

  1. From repository root, run:
    • cd backend
    • golangci-lint run ./pkg/helm/...
    • go build ./pkg/helm/...
    • go test ./pkg/helm/... -count=1
  2. Verify coverage on feature branch:
    • go test ./pkg/helm -coverprofile=/tmp/helm-after.out -count=1
    • go tool cover -func=/tmp/helm-after.out | tail -n 1
  3. (Optional baseline) Compare with main:
    • git checkout main
    • go test ./pkg/helm -coverprofile=/tmp/helm-before.out -count=1
    • go tool cover -func=/tmp/helm-before.out | tail -n 1
    • git checkout backend/helm-repo-error-handling-tests

Notes for the Reviewer

  • Scope is intentionally limited to backend Helm package.
  • No frontend tests/builds were run.
  • Coverage delta is +1.2 percentage points for backend/pkg/helm.

Copilot AI review requested due to automatic review settings April 15, 2026 18:15
@k8s-ci-robot k8s-ci-robot requested review from skoeva and yolossn April 15, 2026 18:15
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 15, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens backend Helm HTTP handler behavior by preventing success responses after error paths, mapping repository delete “not found” to HTTP 404, and adding regression tests to lock in the intended semantics.

Changes:

  • Ensure chart listing returns immediately after an error (no success payload written after http.Error).
  • Return 404 for attempts to delete a non-existent Helm repository, backed by an internal error sentinel.
  • Set Content-Type: application/json before writing status codes for JSON list endpoints and add regression tests for these cases.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
backend/pkg/helm/repository.go Adds errRepositoryNotFound, improves lock error handling, fixes JSON header ordering, and maps repo delete not-found to 404.
backend/pkg/helm/charts.go Returns early on chart list errors and fixes JSON header ordering.
backend/pkg/helm/repository_test.go Adds handler-level regression tests for delete-not-found (404) and JSON content-type.
backend/pkg/helm/repository_internal_test.go Adds internal test coverage for RemoveRepository returning the not-found sentinel error.
backend/pkg/helm/charts_test.go Adds regression test ensuring error responses don’t include a success payload for chart listing.

Comment thread backend/pkg/helm/repository_test.go
Comment thread backend/pkg/helm/repository.go
@Athang69 Athang69 force-pushed the backend/helm-repo-error-handling-tests branch from b9c1fbb to 19b1771 Compare April 16, 2026 03:52
@Athang69 Athang69 requested a review from Copilot April 16, 2026 04:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread backend/pkg/helm/repository_test.go
Comment thread backend/pkg/helm/repository_test.go
Comment thread backend/pkg/helm/repository_test.go
@Athang69 Athang69 force-pushed the backend/helm-repo-error-handling-tests branch from 17dba60 to 09df7ab Compare April 16, 2026 04:51
@Athang69 Athang69 requested a review from Copilot April 16, 2026 04:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread backend/pkg/helm/repository_test.go
@Athang69
Copy link
Copy Markdown
Contributor Author

@sniok @skoeva @illume, I’d appreciate your review on this PR
Thanks

Copy link
Copy Markdown
Contributor

@yolossn yolossn left a comment

Choose a reason for hiding this comment

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

@Athang69 Solid PR, thanks

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Athang69, yolossn

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2026
@yolossn
Copy link
Copy Markdown
Contributor

yolossn commented Apr 16, 2026

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2026
@k8s-ci-robot k8s-ci-robot merged commit e580085 into kubernetes-sigs:main Apr 16, 2026
13 of 14 checks passed
@Athang69
Copy link
Copy Markdown
Contributor Author

Thanks for review @yolossn

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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants