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

Tests for Common/Domain: Adding tests for replication queue message handling and ack update #5730

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

abhishekj720
Copy link
Contributor

Why?:
The recent updates to the unit tests for GetReplicationMessages and UpdateAckLevel functions aim to improve code reliability and maintainability by ensuring that critical functionality within the replication queue's message handling processes is thoroughly validated.
By simulating scenarios of both successful operations and potential failures, these tests help identify issues early in the development cycle, reducing the likelihood of bugs in production.
Additionally, this contributes to the overall effort to increase test coverage for the Cadence service, supporting future development and refactoring with greater confidence.

What has changed?:

  • Added comprehensive test cases for the GetReplicationMessages function to verify both successful message retrieval and proper error handling when message reading fails.

  • Enhanced testing for the UpdateAckLevel function, including scenarios where the ack level update succeeds and fails, ensuring robust error handling and state management.

  • Mock interactions with the QueueManager are fully simulated to ensure the replication queue's logic correctly processes a variety of situations without direct dependency on external systems.

How did you test it?:
Unit tests

Potential risks:
No risks

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Merging #5730 (9c0ed2a) into master (b443f02) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head 9c0ed2a differs from pull request most recent head e35bf55. Consider uploading reports for the commit e35bf55 to get more accurate results

Additional details and impacted files

see 6 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 b443f02...e35bf55. Read the comment docs.

@coveralls
Copy link

coveralls commented Mar 6, 2024

Pull Request Test Coverage Report for Build 018e1498-1238-4b9f-8335-f2d0c5ec5f99

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 55 unchanged lines in 13 files lost coverage.
  • Overall coverage increased (+0.009%) to 63.34%

Files with Coverage Reduction New Missed Lines %
common/persistence/sql/sqlplugin/mysql/db.go 2 79.49%
common/persistence/sql/sqlplugin/mysql/task.go 2 73.68%
common/task/weighted_round_robin_task_scheduler.go 2 89.05%
service/history/task/fetcher.go 2 85.05%
service/history/task/transfer_standby_task_executor.go 2 87.01%
service/matching/db.go 2 73.23%
service/matching/matcher.go 2 90.72%
common/log/tag/tags.go 3 50.18%
common/task/fifo_task_scheduler.go 3 82.47%
service/matching/taskReader.go 5 84.08%
Totals Coverage Status
Change from base Build 018e130e-eb00-4b72-aeca-973c295a2b23: 0.009%
Covered Lines: 92909
Relevant Lines: 146684

💛 - Coveralls

Copy link
Contributor

@3vilhamster 3vilhamster left a comment

Choose a reason for hiding this comment

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

lgtm

@abhishekj720 abhishekj720 enabled auto-merge (squash) March 6, 2024 16:30
@abhishekj720 abhishekj720 merged commit cb39e82 into uber:master Mar 6, 2024
18 checks passed
@abhishekj720 abhishekj720 deleted the testrq2 branch March 6, 2024 17:16
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