Skip to content

tests: Add unit tests for project create command #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rkcoder101 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

tests: Add unit tests for project create command #694

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
db92ef2
test,refactor/refactored CreateProjectCommand() into CreateProject() …
rkcoder101 Feb 9, 2026
e41374f
fix/removed unnecessary fillCreateView function as it was just a wrap…
rkcoder101 Feb 16, 2026
d6c57c6
fix/corrected placed a misplaced comment
rkcoder101 Feb 16, 2026
40f7cde
fix/renamed a variable to avoid unnecessary export from package
rkcoder101 Feb 16, 2026
48f1f4b
fix/improved formatting of error
rkcoder101 Feb 16, 2026
954a418
fix/fixed some very trivial issue in create project tests
rkcoder101 Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 60 additions & 46 deletions cmd/harbor/root/project/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ package project

import (
"fmt"
"io"
"os"

"github.com/goharbor/harbor-cli/pkg/api"
"github.com/goharbor/harbor-cli/pkg/utils"
Expand All @@ -24,52 +26,68 @@ import (
)

// CreateProjectCommand creates a new `harbor create project` command
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

The comment "CreateProjectCommand creates a new harbor create project command" is misplaced. It appears above the ProjectCreator interface definition but actually describes the CreateProjectCommand function further down. Move this comment to line 82 above the CreateProjectCommand function, or update it to describe the ProjectCreator interface instead.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

resolved above

func CreateProjectCommand() *cobra.Command {
var opts create.CreateView
type ProjectCreator interface {
CreateProject(opts create.CreateView) error
FillProjectView(createView *create.CreateView) error
}
type DefaultProjectCreator struct{}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The doc comment above ProjectCreator still says CreateProjectCommand creates ..., but it now documents the interface rather than the command function. This will mislead readers and may trip golint. Move the comment to CreateProjectCommand and add an appropriate comment for ProjectCreator (and/or DefaultProjectCreator).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

ok done 👍


cmd := &cobra.Command{
Use: "create [project name]",
Short: "create project",
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
var err error
var ProjectName string
if len(args) > 0 {
opts.ProjectName = args[0]
}
func (d *DefaultProjectCreator) CreateProject(opts create.CreateView) error {
return api.CreateProject(opts)
}
func (d *DefaultProjectCreator) FillProjectView(createView *create.CreateView) error {
return create.CreateProjectView(createView)
}

if opts.ProxyCache && opts.RegistryID == "" {
return fmt.Errorf("proxy cache selected but no registry ID provided. Use --registry-id")
}
func CreateProject(w io.Writer, projectCreator ProjectCreator, opts *create.CreateView, args []string) error {
var ProjectName string
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

The variable name "ProjectName" uses PascalCase instead of camelCase. Go convention for local variables is to use camelCase. This should be renamed to "projectName" for consistency with Go naming conventions.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

done

var createView *create.CreateView
if len(args) > 0 {
opts.ProjectName = args[0]
Comment on lines 41 to 45
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Local variable ProjectName is capitalized, which in Go signals an exported identifier and is inconsistent with other locals. Rename it to projectName (or inline createView.ProjectName) to follow Go naming conventions and avoid confusion.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

done

}

if !opts.ProxyCache && opts.RegistryID != "" {
return fmt.Errorf("registry ID should only be provided when proxy-cache is enabled")
}
if opts.ProxyCache && opts.RegistryID == "" {
return fmt.Errorf("proxy cache selected but no registry ID provided. Use --registry-id")
}

if opts.ProjectName != "" && opts.StorageLimit != "" {
log.Debug("Attempting to create project using flags...")
err = api.CreateProject(opts)
ProjectName = opts.ProjectName
} else {
log.Debug("Switching to interactive view...")
createView := &create.CreateView{
ProjectName: opts.ProjectName,
Public: opts.Public,
RegistryID: opts.RegistryID,
StorageLimit: opts.StorageLimit,
ProxyCache: opts.ProxyCache,
}
if !opts.ProxyCache && opts.RegistryID != "" {
return fmt.Errorf("registry ID should only be provided when proxy-cache is enabled")
}

err = createProjectView(createView)
ProjectName = createView.ProjectName
}
if opts.ProjectName == "" || opts.StorageLimit == "" {
log.Debug("Switching to interactive view...")
createView = &create.CreateView{
ProjectName: opts.ProjectName,
Public: opts.Public,
RegistryID: opts.RegistryID,
StorageLimit: opts.StorageLimit,
ProxyCache: opts.ProxyCache,
}

if err != nil {
return fmt.Errorf("failed to create project: %v", utils.ParseHarborErrorMsg(err))
}
err := fillCreateView(projectCreator, createView)
if err != nil {
return fmt.Errorf("Failed to get the required params to create project:%w", err)
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Error string "Failed to get the required params to create project:%w" has inconsistent capitalization and is missing a space before the wrapped error. Consider using a lower-case message consistent with the rest of the CLI and formatting like ... project: %w for readability.

Suggested change
return fmt.Errorf("Failed to get the required params to create project:%w", err)
return fmt.Errorf("failed to get the required params to create project: %w", err)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

done

Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

The error message has inconsistent capitalization and formatting. It starts with uppercase "Failed" while other error messages in the same function (lines 50, 54, 77) start with lowercase. Additionally, there should be a space after the colon before %w. Go convention is to use lowercase for error messages unless they start with a proper noun or acronym. Change to: "failed to get the required params to create project: %w"

Suggested change
return fmt.Errorf("Failed to get the required params to create project:%w", err)
return fmt.Errorf("failed to get the required params to create project: %w", err)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

resolved above

}
} else {
createView = opts
}

fmt.Printf("Project '%s' created successfully\n", ProjectName)
return nil
ProjectName = createView.ProjectName
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

The variable name "ProjectName" uses PascalCase instead of camelCase. Go convention for local variables is to use camelCase. This should be renamed to "projectName" for consistency with Go naming conventions.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

done

if err := projectCreator.CreateProject(*createView); err != nil {
return fmt.Errorf("failed to create project: %v", utils.ParseHarborErrorMsg(err))
}
fmt.Fprintf(w, "Project '%s' created successfully\n", ProjectName)
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

The variable declaration can be simplified. The variable ProjectName is declared at line 43 but only assigned at line 75 and used at line 79. Consider removing the declaration at line 43 and directly using createView.ProjectName in the fmt.Fprintf call at line 79, eliminating the unnecessary intermediate variable.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@rkcoder101 rkcoder101 Feb 16, 2026

Choose a reason for hiding this comment

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

dont think this matters much

return nil
}
func CreateProjectCommand() *cobra.Command {
opts := &create.CreateView{}

cmd := &cobra.Command{
Use: "create [project name]",
Short: "create project",
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
return CreateProject(os.Stdout, &DefaultProjectCreator{}, opts, args)
}}

flags := cmd.Flags()
Expand All @@ -81,7 +99,7 @@ func CreateProjectCommand() *cobra.Command {
return cmd
}

func createProjectView(createView *create.CreateView) error {
func fillCreateView(projectCreator ProjectCreator, createView *create.CreateView) error {
if createView == nil {
createView = &create.CreateView{
ProjectName: "",
Expand All @@ -90,10 +108,6 @@ func createProjectView(createView *create.CreateView) error {
StorageLimit: "-1",
}
}

err := create.CreateProjectView(createView)
if err != nil {
return err
}
return api.CreateProject(*createView)
err := projectCreator.FillProjectView(createView)
return err
}
Loading
Loading