tests: Added unit tests for user elevate command by rkcoder101 · Pull Request #686 · goharbor/harbor-cli

rkcoder101 · 2026-02-07T09:10:11Z

Description

This PR introduces unit tests for the user elevate command (cmd/harbor/root/user/elevate.go)

Fixes Incrementally improve unit test coverage #591

Note

No changes have been done to the core logic of the application, only a little refactoring for the purpose of writing some nice and clean test code :)

Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>

codecov · 2026-02-07T09:12:29Z

Codecov Report

❌ Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 7.51%. Comparing base (60ad0bd) to head (526420b).
⚠️ Report is 105 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/harbor/root/user/elevate.go	96.29%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             main    #686      +/-   ##
=========================================
- Coverage   10.99%   7.51%   -3.49%     
=========================================
  Files         173     260      +87     
  Lines        8671   12916    +4245     
=========================================
+ Hits          953     970      +17     
- Misses       7612   11837    +4225     
- Partials      106     109       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR adds unit tests around the user elevate CLI command and refactors the command implementation to be more testable by introducing an injectable dependency interface.

Changes:

Introduces a UserElevator interface + DefaultUserElevator implementation and extracts command logic into ElevateUser(...).
Adds table-driven unit tests covering success and several failure/confirmation flows for ElevateUser, plus a basic ElevateUserCmd() shape test.

Reviewed changes

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

File	Description
`cmd/harbor/root/user/elevate.go`	Refactors elevate command logic into an injectable function to support unit testing.
`cmd/harbor/root/user/elevate_test.go`	Adds unit tests for elevate command logic and command definition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-07T09:13:04Z

cmd/harbor/root/user/elevate.go

 func ElevateUserCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "elevate",
 		Short: "elevate user",
 		Long:  "elevate user to admin role",
 		Args:  cobra.MaximumNArgs(1),
 		Run: func(cmd *cobra.Command, args []string) {
-			var err error
-			var userId int64
-			if len(args) > 0 {
-				userId, err = api.GetUsersIdByName(args[0])
-				if err != nil {
-					log.Errorf("failed to get user id for '%s': %v", args[0], err)
-					return
-				}
-				if userId == 0 {
-					log.Errorf("User with name '%s' not found", args[0])
-					return
-				}
-			} else {
-				userId = prompt.GetUserIdFromUser()
-			}
-			confirm, err := views.ConfirmElevation()
-			if err != nil {
-				log.Errorf("failed to confirm elevation: %v", err)
-				return
-			}
-			if !confirm {
-				log.Error("User did not confirm elevation. Aborting command.")
-				return
-			}
-
-			err = api.ElevateUser(userId)
-			if err != nil {
-				if isUnauthorizedError(err) {
-					log.Error("Permission denied: Admin privileges are required to execute this command.")
-				} else {
-					log.Errorf("failed to elevate user: %v", err)
-				}
-			}
+			d := &DefaultUserElevator{}
+			ElevateUser(args, d)
 		},


The PR description says it adds unit tests for the "user list" command, but the actual changes are for the "user elevate" command (elevate.go / elevate_test.go). Please update the PR description/title to match the scope to avoid confusion when tracking coverage work.

Copilot · 2026-02-07T09:13:04Z

cmd/harbor/root/user/elevate_test.go

+			setup: func() *MockUserElevator {
+				m := initMockUserElevator(5, false, true, nil)
+				m.id["promptuser"] = 999
+				return m


The "elevate user via interactive prompt" test setup writes to m.id["promptuser"], but MockUserElevator.GetUserIDFromUser() always returns 999 and never consults m.id. This line is currently redundant/misleading; either remove it or have the mock's GetUserIDFromUser return a configurable value so the test reflects the intended behavior.

Copilot · 2026-02-07T09:13:04Z

cmd/harbor/root/user/elevate_test.go

+		{
+			name: "user not found logs error",
+			setup: func() *MockUserElevator {
+				return initMockUserElevator(5, false, true, nil)
+			},
+			args:            []string{"nonexistent"},
+			expectedAdminID: []int64{},
+			expectedErr:     "failed to get user id",
+		},
+		{
+			name: "permission denied error",


The tests cover the GetUserIDByName error path, but not the (userID==0, err==nil) "not found" branch that ElevateUser explicitly handles. Since pkg/api.GetUsersIdByName returns (0, nil) when a user is not found, consider adding a test case where GetUserIDByName returns 0 with no error and assert the "User with name ... not found" log message.

Copilot · 2026-02-07T09:13:05Z

cmd/harbor/root/user/elevate.go

+type UserElevator interface {
+	GetUserIDByName(username string) (int64, error)
+	GetUserIDFromUser() int64
+	ConfirmElevation() (bool, error)
+	ElevateUser(userID int64) error
+}
+
+type DefaultUserElevator struct{}
+
+func (d *DefaultUserElevator) GetUserIDByName(username string) (int64, error) {
+	return api.GetUsersIdByName(username)
+}
+
+func (d *DefaultUserElevator) GetUserIDFromUser() int64 {
+	return prompt.GetUserIdFromUser()
+}
+
+func (d *DefaultUserElevator) ConfirmElevation() (bool, error) {
+	return views.ConfirmElevation()
+}
+
+func (d *DefaultUserElevator) ElevateUser(userID int64) error {
+	return api.ElevateUser(userID)
+}
+
+func ElevateUser(args []string, userElevator UserElevator) {
+	var err error
+	var userID int64
+


UserElevator, DefaultUserElevator, and ElevateUser are exported but appear to be used only within this package (and tests are in the same package). Consider making these identifiers unexported (e.g., userElevator/defaultUserElevator/elevateUser) to avoid expanding the public surface area of the cmd package unnecessarily.

Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>

rkcoder101 · 2026-02-07T18:10:15Z

@bupd please review this whenever u get the time. Thanks!

qcserestipy

Thank you for this contribution. The tests work. In general I think adding additional interfaces in the production code for testability only is technical overhead. Please restructure the test to work without this interface.

qcserestipy · 2026-02-16T16:20:16Z

cmd/harbor/root/user/elevate.go

 	"github.com/spf13/cobra"
 )

+type UserElevator interface {


Introducing a UserElevator interface in production code solely for test mockability adds unnecessary complexity. There's only one real implementation (DefaultUserElevator), and this pattern isn't used elsewhere in the codebase. Consider restructuring the tests to avoid altering the production code's API surface, or extracting only the testable logic into a separate function.

Hi @qcserestipy I have tried to adapt a different design of test code. I have tried using global functional variables. Kindly review it, and please let me know if there are any changes required in the approach. Thanks!

… tight coupling of tests Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>

qcserestipy

@rkcoder101 Thank you for incorporating the changes! LGTM, but please fix the lint issues.

rkcoder101 · 2026-02-21T10:12:11Z

@rkcoder101 Thank you for incorporating the changes! LGTM, but please fix the lint issues.

All the PRs opened lately are facing linting issues. Actually there have been some linting issues on the main recently #711 . Fellow contributors have raised PRs to solve them, I will rebase as soon as they get merged.

bupd

https://github.com/goharbor/harbor-cli/actions/runs/22251606412/attempts/1#summary-64376129514

@rkcoder101 Please fix the lint issues - I believe we can make gosec nolint rule on all _test.go files

Please feel free to update it.

test/refactored user elevate cmd and wrote tests for the same

72beb74

Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>

Copilot AI review requested due to automatic review settings February 7, 2026 09:10

Copilot started reviewing on behalf of rkcoder101 February 7, 2026 09:10 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

rkcoder101 mentioned this pull request Feb 7, 2026

test/Unit tests for user command #653

Closed

5 tasks

tests/added a test case suggested in review by copilot

459434b

Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>

qcserestipy requested changes Feb 16, 2026

View reviewed changes

This was referenced Feb 16, 2026

tests,fix: Added unit tests for user password command #687

Open

Incrementally improve unit test coverage #591

Open

changed the test design to avoid using interface for mocking, reduced…

526420b

… tight coupling of tests Signed-off-by: Rayyan Khan <rayyanrehman101@gmail.com>

qcserestipy requested changes Feb 21, 2026

View reviewed changes

bupd requested changes Mar 3, 2026

View reviewed changes

bupd added the Changes Requesed feedback that must be addressed before merging. label Mar 3, 2026

Conversation

rkcoder101 commented Feb 7, 2026 • edited by bupd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Note

Uh oh!

codecov bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

rkcoder101 commented Feb 7, 2026

Uh oh!

qcserestipy left a comment

Choose a reason for hiding this comment

Uh oh!

qcserestipy Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

rkcoder101 Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

qcserestipy left a comment

Choose a reason for hiding this comment

Uh oh!

rkcoder101 commented Feb 21, 2026

Uh oh!

bupd left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rkcoder101 commented Feb 7, 2026 •

edited by bupd

Loading

codecov bot commented Feb 7, 2026 •

edited

Loading