Skip to content
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

added missing unit test for functions in dynamicconfig/config.go #5885

Conversation

d-vignesh
Copy link
Contributor

What changed?
add units tests for functions in dynamicconfig/config.go

Why?
Improve test coverage

How did you test it?
go test

Potential risks

Release notes

Documentation Changes

Copy link

codecov bot commented Apr 6, 2024

Codecov Report

Merging #5885 (804c43c) into master (3a0d600) will decrease coverage by 0.17%.
The diff coverage is n/a.

❗ Current head 804c43c differs from pull request most recent head 0b6a5b9. Consider uploading reports for the commit 0b6a5b9 to get more accurate results

Additional details and impacted files

see 16 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a0d600...0b6a5b9. Read the comment docs.

@coveralls
Copy link

coveralls commented Apr 6, 2024

Pull Request Test Coverage Report for Build 018ebef5-a178-47ea-9bdd-1c168fff5d52

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 46 unchanged lines in 12 files lost coverage.
  • Overall coverage increased (+0.02%) to 67.287%

Files with Coverage Reduction New Missed Lines %
service/matching/matcher.go 1 91.07%
common/task/parallel_task_processor.go 2 93.06%
common/peerprovider/ringpopprovider/config.go 2 81.58%
common/persistence/execution_manager.go 2 83.54%
service/frontend/api/handler.go 2 62.05%
service/history/task/transfer_active_task_executor.go 2 72.7%
common/persistence/statsComputer.go 2 97.14%
common/archiver/filestore/historyArchiver.go 4 80.95%
service/history/task/redispatcher.go 4 89.53%
service/history/execution/mutable_state_task_refresher.go 7 62.03%
Totals Coverage Status
Change from base Build 018ebee7-9c32-494e-9892-ea9da6f92a1a: 0.02%
Covered Lines: 98074
Relevant Lines: 145755

💛 - Coveralls

Copy link
Member

@davidporter-id-au davidporter-id-au left a comment

Choose a reason for hiding this comment

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

In general kindly add to the description all changes, including the refactoring of the contstants file.

Copy link
Contributor

@abhishekj720 abhishekj720 left a comment

Choose a reason for hiding this comment

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

and prefer to write table tests for same methods but different cases as a better testing standard.

@d-vignesh
Copy link
Contributor Author

and prefer to write table tests for same methods but different cases as a better testing standard.

sure, followed this approach as the existing test cases where in this pattern, will follow the table test approach for new tests.

@agautam478 agautam478 enabled auto-merge (squash) April 8, 2024 18:25
@agautam478 agautam478 merged commit 45bf802 into cadence-workflow:master Apr 8, 2024
16 of 18 checks passed
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.

5 participants