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

Add tests for common/persistence/shardManager.go #5916

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

natemort
Copy link
Member

@natemort natemort commented Apr 17, 2024

What changed?
Add tests for common/persistence/shardManager.go
Add mock for ShardStore
Rename shardManager.go to match snake_case convention

Why?
Increase test coverage

How did you test it?
Unit tests

Potential risks

Release notes

Documentation Changes

@coveralls
Copy link

coveralls commented Apr 17, 2024

Pull Request Test Coverage Report for Build 018eee7a-c8d9-4021-b1e7-989fe51bd665

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • 65 unchanged lines in 13 files lost coverage.
  • Overall coverage increased (+0.006%) to 67.487%

Files with Coverage Reduction New Missed Lines %
common/task/weighted_round_robin_task_scheduler.go 1 89.05%
service/history/queue/timer_queue_processor_base.go 1 77.62%
service/history/task/transfer_standby_task_executor.go 2 87.42%
common/task/fifo_task_scheduler.go 2 85.57%
service/frontend/api/handler.go 2 61.92%
common/membership/hashring.go 2 84.69%
service/history/task/transfer_active_task_executor.go 3 72.64%
service/matching/taskListManager.go 3 80.71%
common/persistence/statsComputer.go 3 96.07%
common/archiver/filestore/historyArchiver.go 4 80.95%
Totals Coverage Status
Change from base Build 018eee0e-a88e-4e41-bec1-6a2e488779be: 0.006%
Covered Lines: 98785
Relevant Lines: 146376

💛 - Coveralls

Copy link

codecov bot commented Apr 17, 2024

Codecov Report

❗ No coverage uploaded for pull request base (master@27d99b9). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 3b807b8 differs from pull request most recent head ac24b02. Consider uploading reports for the commit ac24b02 to get more accurate results

Additional details and impacted files
Files Coverage Δ
common/persistence/data_store_interfaces.go 100.00% <ø> (ø)

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 27d99b9...ac24b02. Read the comment docs.

@@ -0,0 +1,399 @@
// The MIT License (MIT)
Copy link
Member

Choose a reason for hiding this comment

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

We are in the process of standardizing file names with snake_case. We update as we touch. Please rename this file and corresponding code file


manager := NewShardManager(store)
if test.serializer != nil {
manager.(*shardManager).serializer = test.serializer
Copy link
Member

Choose a reason for hiding this comment

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

a better alternative pattern for this kind of overrides is to use WithX options. Example

@natemort natemort enabled auto-merge (squash) April 17, 2024 23:59
@natemort natemort merged commit 1c7d522 into cadence-workflow:master Apr 18, 2024
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.

4 participants