test: replace pingcap/check with testify by dveeden · Pull Request #12690 · pingcap/tiflow

dveeden · 2026-06-09T14:05:55Z

What problem does this PR solve?

Issue Number: close #12689

What is changed and how it works?

Migrate all test files using the github.com/pingcap/check (gocheck) framework to testify across the dm/ and pkg/diff/ packages.

Stateless gocheck suites are flattened into plain func TestX(t *testing.T) functions using testify/require.
Stateful suites (with fields and/or SetUp*/TearDown* lifecycle methods) are converted to stretchr/testify/suite, with assertions via s.Require().*, and a suite.Run entry replacing check.TestingT.
c.MkDir() -> t.TempDir(), check.Commentf -> testify message args, check.ErrorMatches -> require.Error + require.Regexp, and the operand order is swapped for Equals/DeepEquals.
conn.InitMockDB now takes require.TestingT instead of *check.C.
github.com/pingcap/check is dropped from go.mod/go.sum.

Co-Authored-By: Claude Opus 4.8 (1M context)

Check List

Tests

Manual test (add detailed scripts or steps below)

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

None

Migrate all test files using the github.com/pingcap/check (gocheck) framework to testify across the dm/ and pkg/diff/ packages. - Stateless gocheck suites are flattened into plain func TestX(t *testing.T) functions using testify/require. - Stateful suites (with fields and/or SetUp*/TearDown* lifecycle methods) are converted to stretchr/testify/suite, with assertions via s.Require().*, and a suite.Run entry replacing check.TestingT. - c.MkDir() -> t.TempDir(), check.Commentf -> testify message args, check.ErrorMatches -> require.Error + require.Regexp, and the operand order is swapped for Equals/DeepEquals. - conn.InitMockDB now takes require.TestingT instead of *check.C. - github.com/pingcap/check is dropped from go.mod/go.sum. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ti-chi-bot · 2026-06-09T14:06:05Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign d3hunter for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS
dm/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dveeden · 2026-06-09T14:06:44Z

/check-issue-triage-complete

gemini-code-assist

Code Review

This pull request migrates the test suite from github.com/pingcap/check to github.com/stretchr/testify across multiple packages, replacing go-check assertions and suites with testify/require and testify/suite. The review feedback correctly identifies a highly dangerous pattern in info_test.go where &testing.T{} is manually instantiated, which can cause nil pointer dereference panics during test execution. Additionally, several instances of swapped expected and actual arguments in require.Equal assertions were identified in source_converter_test.go and task_converters_test.go that should be corrected to ensure accurate test failure messages.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-09T14:10:38Z

+func TestMain(m *testing.M) {
+	mockCluster := integration.NewClusterV3(&testing.T{}, &integration.ClusterConfig{Size: 1})
 	etcdTestCli = mockCluster.RandClient()

-	check.TestingT(t)
+	code := m.Run()
+
+	mockCluster.Terminate(&testing.T{})
+	os.Exit(code)
 }


Passing &testing.T{} to integration.NewClusterV3 and mockCluster.Terminate is highly dangerous. testing.T is managed by the Go testing framework and should never be instantiated manually. An uninitialized testing.T struct has nil internal fields, which will cause a nil pointer dereference panic if any test helper, logging, or cleanup method is called on it.\n\nInstead of using TestMain with a fake testing.T, consider initializing the cluster lazily inside the test functions using the real t *testing.T provided by the test runner, or use a custom testing.TB implementation that safely discards logs and errors.

gemini-code-assist · 2026-06-09T14:10:38Z


 	// we only need to make sure the source config that user can see is the same as the source config that user create
-	c.Assert(sourceCfg1.String(), check.Equals, sourceCfg2.String())
+	require.Equal(t, sourceCfg2.String(), sourceCfg1.String())


The arguments to require.Equal are swapped. The expected value should be the first argument (sourceCfg1.String()), and the actual value should be the second argument (sourceCfg2.String()). Swapping them ensures that test failure messages are accurate and easy to read.

Suggested change

require.Equal(t, sourceCfg2.String(), sourceCfg1.String())

\trequire.Equal(t, sourceCfg1.String(), sourceCfg2.String())

gemini-code-assist · 2026-06-09T14:10:38Z

 	openapiSource2 := SourceCfgToOpenAPISource(OpenAPISourceToSourceCfg(openapiSource1))
 	openapiSource2.Password = openapiSource1.Password // we set passwd to "******" for privacy
-	c.Assert(openapiSource1, check.DeepEquals, openapiSource2)
+	require.Equal(t, openapiSource2, openapiSource1)


The arguments to require.Equal are swapped. The expected value should be the first argument (openapiSource1), and the actual value should be the second argument (openapiSource2). Swapping them ensures that test failure messages are accurate and easy to read.

Suggested change

require.Equal(t, openapiSource2, openapiSource1)

\trequire.Equal(t, openapiSource1, openapiSource2)

gemini-code-assist · 2026-06-09T14:10:38Z

+	require.Len(t, taskList, 1)
 	newTask := taskList[0]
-	c.Assert(&task, check.DeepEquals, newTask)
+	require.Equal(t, newTask, &task)


The arguments to require.Equal are swapped. The expected value should be the first argument (&task), and the actual value should be the second argument (newTask). Swapping them ensures that test failure messages are accurate and easy to read.

Suggested change

require.Equal(t, newTask, &task)

\trequire.Equal(t, &task, newTask)

gemini-code-assist · 2026-06-09T14:10:38Z

 	}

-	c.Assert(&task, check.DeepEquals, newTask)
+	require.Equal(t, newTask, &task)


The arguments to require.Equal are swapped. The expected value should be the first argument (&task), and the actual value should be the second argument (newTask). Swapping them ensures that test failure messages are accurate and easy to read.

Suggested change

require.Equal(t, newTask, &task)

\trequire.Equal(t, &task, newTask)

…heck-with-testify

dveeden · 2026-06-11T13:29:45Z

/retest

dveeden · 2026-06-11T15:58:23Z

/retest

Fix the failures that pull-verify found in dm_unit_test_in_verify_ci: - dm/master, dm/relay: patterns with a leading bare '*' compiled under pingcap/check because ErrorMatches prepends '^', but panic in require.Regexp which uses the pattern verbatim. Anchor with '.*'. - dm/pkg/shardddl/optimism: parse the testing flags in TestMain before integration.NewClusterV3, which consults testing.Short and panics if flags are not parsed yet. - dm/syncer/dbconn: skip the suite when no upstream MySQL is reachable. The check.Suite was never run (no check.TestingT in the package), so the migrated suite ran for the first time in CI and tried to connect to a real MySQL on 127.0.0.1:3306. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

dveeden · 2026-06-12T09:46:42Z

/test pull-error-log-review

wuhuizuo · 2026-06-12T10:01:09Z

/override pull-error-log-review

ti-chi-bot · 2026-06-12T10:01:12Z

@wuhuizuo: wuhuizuo unauthorized: /override is restricted to Repo administrators, and the following github teams:.

Details

In response to this:

/override pull-error-log-review

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

wuhuizuo · 2026-06-12T10:47:56Z

/override pull-error-log-review

ti-chi-bot · 2026-06-12T10:48:02Z

@wuhuizuo: Overrode contexts on behalf of wuhuizuo: pull-error-log-review

Details

In response to this:

/override pull-error-log-review

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

dveeden · 2026-06-12T10:55:49Z

/cc @OliverS929 @kennytm

dveeden · 2026-06-16T09:10:39Z

/retest

ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed area/dm Issues or PRs related to DM. labels Jun 9, 2026

ti-chi-bot Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 9, 2026

ti-chi-bot Bot removed the do-not-merge/needs-triage-completed label Jun 9, 2026

gemini-code-assist Bot reviewed Jun 9, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/master' into replace-pingcap-c…

fcdaecd

…heck-with-testify

ti-chi-bot Bot requested review from OliverS929 and kennytm June 12, 2026 10:55

	require.Equal(t, sourceCfg2.String(), sourceCfg1.String())
	\trequire.Equal(t, sourceCfg1.String(), sourceCfg2.String())

	require.Equal(t, openapiSource2, openapiSource1)
	\trequire.Equal(t, openapiSource1, openapiSource2)

	require.Equal(t, newTask, &task)
	\trequire.Equal(t, &task, newTask)

Conversation

dveeden commented Jun 9, 2026

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

Uh oh!

ti-chi-bot Bot commented Jun 9, 2026

Uh oh!

dveeden commented Jun 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

dveeden commented Jun 11, 2026

Uh oh!

dveeden commented Jun 11, 2026

Uh oh!

dveeden commented Jun 12, 2026

Uh oh!

wuhuizuo commented Jun 12, 2026

Uh oh!

ti-chi-bot Bot commented Jun 12, 2026

Uh oh!

wuhuizuo commented Jun 12, 2026

Uh oh!

ti-chi-bot Bot commented Jun 12, 2026

Uh oh!

dveeden commented Jun 12, 2026

Uh oh!

dveeden commented Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants