Skip to content

Unit testing Coverage report#376

Merged
qcserestipy merged 25 commits into
goharbor:mainfrom
qcserestipy:testing_coverage
May 23, 2025
Merged

Unit testing Coverage report#376
qcserestipy merged 25 commits into
goharbor:mainfrom
qcserestipy:testing_coverage

Conversation

@qcserestipy

@qcserestipy qcserestipy commented Apr 4, 2025

Copy link
Copy Markdown
Collaborator

Draft PR: Enhancing Unit Testing and Code Coverage Reporting

This PR is a work-in-progress aimed at discussing and refining our approach to unit testing and code coverage in the project. Below is an overview of the key changes introduced:

  • Unit Test Coverage with Dagger:
    I have added Dagger functions that generate a unit test coverage report for the repository. This setup allows us to execute tests in a containerized environment and obtain comprehensive coverage metrics.

  • Reorganized Test Structure:
    Our current tests were more aligned with unit testing rather than end-to-end (E2E) testing. To improve clarity and maintainability, I have moved these tests into distinct packages within the cmd and pkg directories. This separation will help us better manage and expand our test suites.

  • Shared Testing Helper Package:
    A new testing helper package has been created to consolidate functionality shared by all tests. This change reduces duplication and makes it easier to write and maintain future tests.

Since additional unit tests are expected in forthcoming PRs, this PR serves as a preliminary framework. I would also like to open a discussion on how best to integrate unit testing and reporting into our CI/CD pipeline. For example, we could eventually add a coverage report badge to the project to provide ongoing visibility into our code quality.

I look forward to your feedback and suggestions.

Decisions Required Before Merging

  • Codecov Badge: Replace my personal Codecov badge in README with the official goharbor/harbor-cli badge
  • Coverage Summary Format: Confirm the current GitHub Actions summary style is appropriate (emoji indicators, collapsible details)
  • CODECOV_TOKEN: Verify the CODECOV_TOKEN secret is properly configured in the repository settings
  • Coverage Thresholds: Decide if we want to enforce minimum coverage requirements (currently set to 80% target)

Screenshots

Quick View


Screenshot 2025-05-20 at 20 27 52

Extended view

Screenshot 2025-05-20 at 20-29-59 Draft Unit testing Coverage report · goharbor_harbor-cli@3045aa6

Codecov commit view

Screenshot 2025-05-20 at 20-42-16 Codecov

@qcserestipy

Copy link
Copy Markdown
Collaborator Author

As a test I also added the codecov badge to the README in my own branch:
Screenshot 2025-04-04 at 17 35 08

This uses my own codecov account, but maybe the goharbor one could be used.

@qcserestipy qcserestipy force-pushed the testing_coverage branch 2 times, most recently from 588aeaa to 2cffa73 Compare May 13, 2025 13:25
@qcserestipy qcserestipy added the enhancement New feature or request label May 13, 2025
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
add coverage threshold verification to dagger function
integrate coverage check into GitHub Actions pipeline
ensure proper syntax in shell script for accurate comparison
set initial coverage threshold at 80% for CI enforcement

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
…lso in memeory config can be updated

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
@codecov

codecov Bot commented May 20, 2025

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
…onfig cmd test from to context_test pkg

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
@qcserestipy qcserestipy marked this pull request as ready for review May 20, 2025 18:33
@qcserestipy

Copy link
Copy Markdown
Collaborator Author

Now this PR is ready for review. I've updated the description with all the details about the test coverage implementation and undrafted it. @Vad1mo, I'd appreciate your feedback, especially on the "Decisions Required Before Merging" section.

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
@qcserestipy qcserestipy changed the title Draft: Unit testing Coverage report Unit testing Coverage report May 20, 2025
@Vad1mo Vad1mo requested a review from Copilot May 23, 2025 11:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This draft PR establishes a framework for unit testing and coverage reporting, reorganizes existing tests into focused packages, and centralizes shared test utilities.

  • Integrate Dagger-based test coverage and reporting tasks into CI
  • Reorganize tests under utils_test and root_test packages
  • Introduce a shared test/helper package and new SetCurrentHarborConfig API

Reviewed Changes

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

Show a summary per file
File Description
pkg/utils/helper_test.go Add end-to-end unit tests for utility functions
pkg/utils/encryption_test.go Rename test package to utils_test
pkg/utils/config_test.go Refactor config tests to use shared helpers
pkg/utils/config.go Add SetCurrentHarborConfig method
pkg/utils/client_test.go Add tests for client initialization
pkg/utils/client.go Rename client globals and improve error logging
pkg/api/project_handler.go Add error handling for ParseProjectRepo in deletion
cmd/harbor/root/repository/view.go Handle parse errors in RepoViewCmd
cmd/harbor/root/repository/delete.go Handle parse errors and early return in RepoDeleteCmd
cmd/harbor/root/login_test.go Refactor login tests to use shared helpers
cmd/harbor/root/context/config_cmd_test.go Refactor context commands tests using helpers
cmd/harbor/root/artifact/view.go Add parse error check in ViewArtifactCommand
cmd/harbor/root/artifact/tags.go Add parse error check in tag creation/list/delete cmds
cmd/harbor/root/artifact/scan.go Add parse error check in scan start/stop commands
cmd/harbor/root/artifact/list.go Validate parse results in ListArtifactsCmd
cmd/harbor/root/artifact/delete.go Add parse error check in DeleteArtifactCommand
README.md Add a personal Codecov badge (needs replacement)
.github/workflows/default.yaml Add Dagger coverage and Codecov steps
.dagger/main.go Add TestCoverage and TestCoverageReport steps
.dagger/README.md Document new coverage-report commands
Comments suppressed due to low confidence (2)

pkg/api/project_handler.go:119

  • The variable projectName is not defined in this scope. It should use the incoming projectNameOrID or introduce a local projectName before calling RepoDelete.
err = RepoDelete(projectName, repoName, useProjectID)

cmd/harbor/root/artifact/list.go:56

  • This callback is defined with Run: (no return value). Returning an error here will not compile. Use RunE: or handle the error via cmd.PrintErr and os.Exit(1).
return fmt.Errorf("failed to parse project/repo: %v", err)

Comment thread cmd/harbor/root/repository/delete.go
Comment thread README.md Outdated
Comment thread .dagger/README.md Outdated
Comment thread pkg/utils/client.go
Comment thread pkg/utils/client.go Outdated
qcserestipy and others added 4 commits May 23, 2025 14:27
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
@qcserestipy qcserestipy merged commit 2d67810 into goharbor:main May 23, 2025
9 checks passed
rizul2108 pushed a commit to rizul2108/harbor-cli that referenced this pull request May 24, 2025
* Resolve merge conflicts

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added codecov badge for testing

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added codecov badge for testing

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added helpers package to context test after upstream rebase

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added changes to satisfy linter

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added dagger coverage steps to pipeline

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* ci(coverage): implement test coverage threshold check

add coverage threshold verification to dagger function
integrate coverage check into GitHub Actions pipeline
ensure proper syntax in shell script for accurate comparison
set initial coverage threshold at 80% for CI enforcement

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Fix failing test

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Cleanup client testing; added setconfig function to utils such that also in memeory config can be updated

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added changes to satisfy linter

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added test coverage entries to dagger readme

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added test coverage entries to dagger readme

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Test code cov token for upload

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Fix: wrong helper import in cmd test

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Update: test coverage report export

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Changed pipeline for test summary

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Changed pipeline for test summary

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Changed pipeline for test summary

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Adjusted dagger function for test report; added step summary; moved config cmd test from to context_test pkg

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added note about target coverage

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Readded coverage step for codecov upload

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Update cmd/harbor/root/repository/delete.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

* Update README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

* Update .dagger/README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

* Update pkg/utils/client.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

---------

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Darkhood148 pushed a commit to Darkhood148/harbor-cli that referenced this pull request May 27, 2025
* Resolve merge conflicts

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added codecov badge for testing

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added codecov badge for testing

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added helpers package to context test after upstream rebase

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added changes to satisfy linter

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added dagger coverage steps to pipeline

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* ci(coverage): implement test coverage threshold check

add coverage threshold verification to dagger function
integrate coverage check into GitHub Actions pipeline
ensure proper syntax in shell script for accurate comparison
set initial coverage threshold at 80% for CI enforcement

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Fix failing test

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Cleanup client testing; added setconfig function to utils such that also in memeory config can be updated

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added changes to satisfy linter

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added test coverage entries to dagger readme

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added test coverage entries to dagger readme

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Test code cov token for upload

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Fix: wrong helper import in cmd test

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Update: test coverage report export

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Changed pipeline for test summary

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Changed pipeline for test summary

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Changed pipeline for test summary

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Adjusted dagger function for test report; added step summary; moved config cmd test from to context_test pkg

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Added note about target coverage

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Readded coverage step for codecov upload

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

* Update cmd/harbor/root/repository/delete.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

* Update README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

* Update .dagger/README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

* Update pkg/utils/client.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>

---------

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants