Skip to content

Conversation

@LucienShen-Liu
Copy link
Contributor

What this PR does:improve test coverage for pkg/datasource/sql/undo

Which issue(s) this PR fixes: #940

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive unit test coverage for the undo logging functionality in the SQL datasource package. The tests validate manager registration, builder patterns, executor interfaces, and configuration handling for the undo log system.

  • Unit tests added for core undo functionality including manager, builder, and executor interfaces
  • Configuration and compression handling test coverage
  • MySQL-specific undo log manager implementation tests

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/datasource/sql/undo/undo_test.go Tests for undo log manager registration, builder patterns, and core data structures
pkg/datasource/sql/undo/undo_executor_test.go Tests for undo executor interface implementation
pkg/datasource/sql/undo/undo_executor_holder_test.go Tests for undo executor holder interface and factory methods
pkg/datasource/sql/undo/mysql/undo_test.go Tests for MySQL-specific undo log manager implementation
pkg/datasource/sql/undo/mysql/default_test.go Tests for MySQL undo log manager initialization
pkg/datasource/sql/undo/factor/undo_executor_holder_factor_test.go Tests for undo executor holder factory with lazy initialization
pkg/datasource/sql/undo/factor/undo_executor_factory_test.go Tests for undo executor factory and SQL type routing
pkg/datasource/sql/undo/config_test.go Tests for undo configuration and flag registration
pkg/datasource/sql/undo/base/undo_test.go Tests for base undo log manager utility functions
go.mod Updated to include testify mock dependency
go.sum Checksum for testify objx dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}{
{
name: "can undo - normal status",
logStatus: UndoLogStatueNormnal,
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'UndoLogStatueNormnal' to 'UndoLogStatusNormal'. The constant name contains typo 'Statue' instead of 'Status' and 'Normnal' instead of 'Normal'.

Suggested change
logStatus: UndoLogStatueNormnal,
logStatus: UndoLogStatusNormal,

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue was a spelling error in the original constant name; it has now been corrected.

Comment on lines 220 to 230
logStatus UndoLogStatue
want bool
}{
{
name: "can undo - normal status",
logStatus: UndoLogStatueNormnal,
want: true,
},
{
name: "cannot undo - global finished",
logStatus: UndoLogStatueGlobalFinished,
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'UndoLogStatue' to 'UndoLogStatus'. The type name contains typo 'Statue' instead of 'Status'.

Suggested change
logStatus UndoLogStatue
want bool
}{
{
name: "can undo - normal status",
logStatus: UndoLogStatueNormnal,
want: true,
},
{
name: "cannot undo - global finished",
logStatus: UndoLogStatueGlobalFinished,
logStatus UndoLogStatus
want bool
}{
{
name: "can undo - normal status",
logStatus: UndoLogStatusNormal,
want: true,
},
{
name: "cannot undo - global finished",
logStatus: UndoLogStatusGlobalFinished,

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue was a spelling error in the original constant name; it has now been corrected.

},
{
name: "cannot undo - global finished",
logStatus: UndoLogStatueGlobalFinished,
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'UndoLogStatueGlobalFinished' to 'UndoLogStatusGlobalFinished'. The constant name contains typo 'Statue' instead of 'Status'.

Suggested change
logStatus: UndoLogStatueGlobalFinished,
logStatus: UndoLogStatusGlobalFinished,

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue was a spelling error in the original constant name; it has now been corrected.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2025

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.90%. Comparing base (4420143) to head (7888315).

Files with missing lines Patch % Lines
pkg/datasource/sql/undo/base/undo.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #971      +/-   ##
==========================================
- Coverage   46.33%   45.90%   -0.43%     
==========================================
  Files         195      200       +5     
  Lines       11887    12264     +377     
==========================================
+ Hits         5508     5630     +122     
- Misses       5943     6196     +253     
- Partials      436      438       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Move manager variable creation after t.Skip() calls in MySQL undo tests
to prevent unused variable warnings from golangci-lint staticcheck.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@Code-Fight
Copy link
Contributor

@LucienShen-Liu pls resolve conflicts

@LucienShen-Liu
Copy link
Contributor Author

@LucienShen-Liu pls resolve conflicts

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants