Skip to content

[GoFr SOC] Improve Unit Test Coverage Across 6 Core Packages (zip, logger, pubsub/google, etc.) #2109

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.

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
wants to merge 23 commits into
base: development
Choose a base branch
from

Conversation

Ashish-Kumar-Dash
Copy link

@Ashish-Kumar-Dash Ashish-Kumar-Dash commented Jul 29, 2025

Description:

Added and enhanced unit tests to improve test coverage for the following packages:

pkg/gofr/file/zip.go

pkg/gofr/logging/logger.go

pkg/gofr/datasource/pubsub/google

pkg/gofr/migration/sql.go

pkg/gofr/datasource/sql/db.go

pkg/gofr/http/form_data_binder.go

Addressed edge cases, nil checks, and error handling branches that were previously untested.

Motivation: Boost reliability and confidence in core utilities and data layer by ensuring critical paths are covered.

Related to ongoing gofr SOC coverage enhancement initiative.

Coverage Summary:
File: file/zip.go — Before: 79.1% | After: 88.4% | Change: +9.3%

File: logging/logger.go — Before: 84.5% | After: 88.5% | Change: +4.0%

File: datasource/pubsub/google — Before: ~41.9% | After: 43.8% | Change: +1.9%

File: migration/sql.go — Before: 38.1% | After: 81.7% | Change: +43.6%

File: datasource/sql/db.go — Before: 55.6% | After: 91.2% | Change: +35.6%

File: http/form_data_binder.go — Added form_data_binder_test.go to cover core logic

Breaking Changes:

None introduced. All enhancements are test-only and non-breaking.

Additional Information:

Dependencies used:

pstest for in-memory Pub/Sub testing

testify for assertions

gomock for mocking interfaces

No production code changed. All changes are isolated to test files.

Coverage measured using go test -coverprofile and go tool cover -func.

Checklist:

[x] I have formatted my code using goimport and golangci-lint.
[x] All new code is covered by unit tests.
[x] This PR does not decrease the overall code coverage.
[x] I have reviewed the code comments and documentation for clarity.
@Umang01-hash kindly review and advise on any changes, if needed
Thank You

@Ashish-Kumar-Dash
Copy link
Author

hey @Umang01-hash @coolwednesday , can you kindly comment on the next course of action? Should i resolve the Workflow-Pipeline / Code Quality🎖️ (pull_request) check?

Copy link
Member

@Umang01-hash Umang01-hash left a comment

Choose a reason for hiding this comment

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

Hey @up1 Thankyou for your contribution. There are a few linter as well as test failures in your PR. Please fix them!

@Ashish-Kumar-Dash
Copy link
Author

hey @Umang01-hash I have fixed the linting errors & the errors in the test cases.I have modified the logic of some test cases,and the ones I haven't been able to resolve have been replaced by new test cases that keep the updated level of coverage ~same as shown in my PR description. Kindly review and let me know if any changes are needed,
Thank you

@coolwednesday
Copy link
Member

@Ashish-Kumar-Dash , Would like you to fix the linters .... a quick way to get them fixed is to install golangci-lint locally so that you can lint the files before pushing the code and prevent this repetitive issue.

@Ashish-Kumar-Dash
Copy link
Author

Ashish-Kumar-Dash commented Aug 8, 2025

hey @coolwednesday @Umang01-hash Regarding the lint issues,on first seeing the gci error i ran the gci tool for formatting but after i pushed new errors came up of extra white space and white lines,so, for past 2 commits i have run both golangci-lint run & golangci-lint run --output.text.print-issued-lines --output.text.colors=true --show-stats=false --timeout=5m(lint root module test command), and they have passed them locally,but always there is some hidden error Github Workflows finds which when i run the same test on my machine it doesn't catch it. It has been kind of frustrating, im attaching the ss of tests i ran of the most recent commit (fix lint issues) and you can see no error was flagged.
Screenshot 2025-08-08 180708
If Github Workflows catch a linting error now again then I will be keen to hear what else I can do, which I haven't previously tried, because seeing through SS you can see golangci-lint caught no error.
Thank You

@coolwednesday
Copy link
Member

hey @coolwednesday @Umang01-hash Regarding the lint issues,on first seeing the gci error i ran the gci tool for formatting but after i pushed new errors came up of extra white space and white lines,so, for past 2 commits i have run both golangci-lint run & golangci-lint run --output.text.print-issued-lines --output.text.colors=true --show-stats=false --timeout=5m(lint root module test command), and they have passed them locally,but always there is some hidden error Github Workflows finds which when i run the same test on my machine it doesn't catch it. It has been kind of frustrating, im attaching the ss of tests i ran of the most recent commit (fix lint issues) and you can see no error was flagged. Screenshot 2025-08-08 180708 If Github Workflows catch a linting error now again then I will be keen to hear what else I can do, which I haven't previously tried, because seeing through SS you can see golangci-lint caught no error. Thank You

The error in the screenshot is the error I see on Code quality job in workflow.

Copy link
Member

@Umang01-hash Umang01-hash left a comment

Choose a reason for hiding this comment

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

  • Please remove the go.work.sum changes as it is not related to your PR. Else apart the PR looks fine and can be merged. And regarding the linter issues i just fixed it for you. Sometimes the version of linter and all creates these situations where in your local you are not getting any warnings but on github pipeline it shows linter errors. I think one thing you must make sure that linter versions in your local as well as one used in workflow are same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants