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.

Sign up for GitHub

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

test(pkg/version): migrate test-infra to testify for pkg/version pkg #2920

Merged
merged 6 commits into from
Oct 2, 2021
Merged

test(pkg/version): migrate test-infra to testify for pkg/version pkg #2920

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eec79da
test(pkg/version): migrate test-infra to testify for pkg/version pkg
Sep 26, 2021
2390d28
test(pkg/version): replace c.Error() with assert.Fail(t)
Sep 26, 2021
c8dfc65
Merge branch 'master' into pkg-version-testify
Sep 29, 2021
29961b0
remove blank line
amyangfei Oct 2, 2021
415e367
Merge branch 'master' into pkg-version-testify
ti-chi-bot Oct 2, 2021
3a16792
Merge branch 'master' into pkg-version-testify
ti-chi-bot Oct 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 64 additions & 75 deletions pkg/version/check_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,19 @@ import (
"fmt"
"net/http"
"net/url"
"os"
"testing"
"time"

"github.com/coreos/go-semver/semver"
"github.com/pingcap/check"
"github.com/pingcap/kvproto/pkg/metapb"
"github.com/pingcap/ticdc/cdc/model"
"github.com/pingcap/ticdc/pkg/util/testleak"
"github.com/stretchr/testify/require"

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/require"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

??? what's the difference?

Copy link
Member

Choose a reason for hiding this comment

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

@overvenus He should have meant the empty line below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, it's a problem adding an empty line here?

Copy link
Member

Choose a reason for hiding this comment

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

See: https://github.com/pingcap/ticdc/pull/2920#discussion_r716200957

Copy link
Contributor

@amyangfei amyangfei Oct 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

removed blank line

pd "github.com/tikv/pd/client"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"github.com/stretchr/testify/require"
pd "github.com/tikv/pd/client"
"github.com/stretchr/testify/require"
pd "github.com/tikv/pd/client"

"github.com/tikv/pd/pkg/tempurl"
)

func Test(t *testing.T) {
check.TestingT(t)
}

type checkSuite struct{}

var _ = check.Suite(&checkSuite{})

type mockPDClient struct {
pd.Client
getAllStores func() []*metapb.Store
Expand All @@ -63,8 +56,8 @@ func (m *mockPDClient) ServeHTTP(resp http.ResponseWriter, _ *http.Request) {
}
}

func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
defer testleak.AfterTest(c)()
func TestCheckClusterVersion(t *testing.T) {
t.Parallel()
mock := mockPDClient{
Client: nil,
}
Expand All @@ -83,9 +76,9 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
if err == nil {
break
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so what's the point for sleep in loop to wait? cause the retry already means fail when call c.Error()

c.Error(err)
fmt.Fprintf(os.Stderr, "[ERROR]:%v", err)
Copy link
Member

Choose a reason for hiding this comment

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

How about we use t.Log(f)?

Also the original c.Error should make the case fail straight away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(1) t.Log(f) LGTM.
(2) c.Error will fail the test but not stop the test immediately. So i can't find the corresponding function in testify like c.Error. testify has fail/failnow/failf function, but they will stop the test-case directly. Any suggestions? @hi-rustin

Copy link
Member

Choose a reason for hiding this comment

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

If it's all going to fail, then I see nothing wrong with ending it early. @overvenus What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find the testify.assert.Fail can act just like c.Error(). But the semantic of assert in testify still confuses me, cause assert means severe mismatch in C/C++.

Copy link
Member

Choose a reason for hiding this comment

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

😆 Cool!

if i == 19 {
c.Fatal("http server timeout", err)
require.FailNowf(t, "TestCheckClusterVersion fail", "http server timeout:%v", err)
}
}

Expand All @@ -97,7 +90,7 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
return []*metapb.Store{{Version: MinTiKVVersion.String()}}
}
err := CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, true)
c.Assert(err, check.IsNil)
require.Nil(t, err)
}

{
Expand All @@ -108,7 +101,7 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
return []*metapb.Store{{Version: MinTiKVVersion.String()}}
}
err := CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, true)
c.Assert(err, check.ErrorMatches, ".*PD .* is not supported.*")
require.Regexp(t, ".*PD .* is not supported.*", err)
}

// Check maximum compatible PD.
Expand All @@ -120,7 +113,7 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
return []*metapb.Store{{Version: MinTiKVVersion.String()}}
}
err := CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, true)
c.Assert(err, check.ErrorMatches, ".*PD .* is not supported.*")
require.Regexp(t, ".*PD .* is not supported.*", err)
}

{
Expand All @@ -132,9 +125,9 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
return []*metapb.Store{{Version: `1.0.0-alpha-271-g824ae7fd`}}
}
err := CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, true)
c.Assert(err, check.ErrorMatches, ".*TiKV .* is not supported.*")
require.Regexp(t, ".*TiKV .* is not supported.*", err)
err = CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, false)
c.Assert(err, check.IsNil)
require.Nil(t, err)
}

// Check maximum compatible TiKV.
Expand All @@ -147,7 +140,7 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
return []*metapb.Store{{Version: `10000.0.0`}}
}
err := CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, true)
c.Assert(err, check.ErrorMatches, ".*TiKV .* is not supported.*")
require.Regexp(t, ".*TiKV .* is not supported.*", err)
}

{
Expand All @@ -156,31 +149,29 @@ func (s *checkSuite) TestCheckClusterVersion(c *check.C) {
}

err := CheckClusterVersion(context.Background(), &mock, pdAddrs, nil, false)
c.Assert(err, check.ErrorMatches, ".*response status: .*")
require.Regexp(t, ".*response status: .*", err)
}
}

func (s *checkSuite) TestCompareVersion(c *check.C) {
defer testleak.AfterTest(c)()
c.Assert(semver.New("4.0.0-rc").Compare(*semver.New("4.0.0-rc.2")), check.Equals, -1)
c.Assert(semver.New("4.0.0-rc.1").Compare(*semver.New("4.0.0-rc.2")), check.Equals, -1)
c.Assert(semver.New(removeVAndHash("4.0.0-rc-35-g31dae220")).Compare(*semver.New("4.0.0-rc.2")), check.Equals, -1)
c.Assert(semver.New(removeVAndHash("4.0.0-9-g30f0b014")).Compare(*semver.New("4.0.0-rc.1")), check.Equals, 1)

c.Assert(semver.New(removeVAndHash("4.0.0-rc-35-g31dae220")).Compare(*semver.New("4.0.0-rc.2")), check.Equals, -1)
c.Assert(semver.New(removeVAndHash("4.0.0-9-g30f0b014")).Compare(*semver.New("4.0.0-rc.1")), check.Equals, 1)
c.Assert(semver.New(removeVAndHash("v3.0.0-beta-211-g09beefbe0-dirty")).
Compare(*semver.New("3.0.0-beta")), check.Equals, 0)
c.Assert(semver.New(removeVAndHash("v3.0.5-dirty")).
Compare(*semver.New("3.0.5")), check.Equals, 0)
c.Assert(semver.New(removeVAndHash("v3.0.5-beta.12-dirty")).
Compare(*semver.New("3.0.5-beta.12")), check.Equals, 0)
c.Assert(semver.New(removeVAndHash("v2.1.0-rc.1-7-g38c939f-dirty")).
Compare(*semver.New("2.1.0-rc.1")), check.Equals, 0)
func TestCompareVersion(t *testing.T) {
require.Equal(t, semver.New("4.0.0-rc").Compare(*semver.New("4.0.0-rc.2")), -1)
require.Equal(t, semver.New("4.0.0-rc.1").Compare(*semver.New("4.0.0-rc.2")), -1)
require.Equal(t, semver.New(removeVAndHash("4.0.0-rc-35-g31dae220")).Compare(*semver.New("4.0.0-rc.2")), -1)
require.Equal(t, semver.New(removeVAndHash("4.0.0-9-g30f0b014")).Compare(*semver.New("4.0.0-rc.1")), 1)

require.Equal(t, semver.New(removeVAndHash("4.0.0-rc-35-g31dae220")).Compare(*semver.New("4.0.0-rc.2")), -1)
require.Equal(t, semver.New(removeVAndHash("4.0.0-9-g30f0b014")).Compare(*semver.New("4.0.0-rc.1")), 1)
require.Equal(t, semver.New(removeVAndHash("v3.0.0-beta-211-g09beefbe0-dirty")).
Compare(*semver.New("3.0.0-beta")), 0)
require.Equal(t, semver.New(removeVAndHash("v3.0.5-dirty")).
Compare(*semver.New("3.0.5")), 0)
require.Equal(t, semver.New(removeVAndHash("v3.0.5-beta.12-dirty")).
Compare(*semver.New("3.0.5-beta.12")), 0)
require.Equal(t, semver.New(removeVAndHash("v2.1.0-rc.1-7-g38c939f-dirty")).
Compare(*semver.New("2.1.0-rc.1")), 0)
}

func (s *checkSuite) TestReleaseSemver(c *check.C) {
defer testleak.AfterTest(c)()
func TestReleaseSemver(t *testing.T) {
cases := []struct{ releaseVersion, releaseSemver string }{
{"None", ""},
{"HEAD", ""},
Expand All @@ -190,12 +181,12 @@ func (s *checkSuite) TestReleaseSemver(c *check.C) {

for _, cs := range cases {
ReleaseVersion = cs.releaseVersion
c.Assert(ReleaseSemver(), check.Equals, cs.releaseSemver, check.Commentf("%v", cs))
require.Equal(t, ReleaseSemver(), cs.releaseSemver, "%v", cs)
}
}

func (s *checkSuite) TestGetTiCDCClusterVersion(c *check.C) {
defer testleak.AfterTest(c)()
func TestGetTiCDCClusterVersion(t *testing.T) {
t.Parallel()
testCases := []struct {
captureInfos []*model.CaptureInfo
expected TiCDCClusterVersion
Expand Down Expand Up @@ -247,61 +238,59 @@ func (s *checkSuite) TestGetTiCDCClusterVersion(c *check.C) {
}
for _, tc := range testCases {
ver, err := GetTiCDCClusterVersion(tc.captureInfos)
c.Assert(err, check.IsNil)
c.Assert(ver, check.DeepEquals, tc.expected)
require.Nil(t, err)
require.Equal(t, ver, tc.expected)
}
}

func (s *checkSuite) TestTiCDCClusterVersionFeaturesCompatible(c *check.C) {
defer testleak.AfterTest(c)()

func TestTiCDCClusterVersionFeaturesCompatible(t *testing.T) {
t.Parallel()
ver := TiCDCClusterVersion{semver.New("4.0.10")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, false)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, false)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), false)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), false)

ver = TiCDCClusterVersion{semver.New("4.0.12")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, false)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, false)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), false)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), false)

ver = TiCDCClusterVersion{semver.New("4.0.13")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, false)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), false)

ver = TiCDCClusterVersion{semver.New("4.0.13-hotfix")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, false)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), false)

ver = TiCDCClusterVersion{semver.New("4.0.14")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, false)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), false)

ver = TiCDCClusterVersion{semver.New("5.0.0-rc")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, false)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, true)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), false)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), true)

ver = TiCDCClusterVersion{semver.New("5.0.0")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, true)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), true)

ver = TiCDCClusterVersion{semver.New("5.1.0")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, true)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), true)

ver = TiCDCClusterVersion{semver.New("5.2.0-alpha")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, true)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), true)

ver = TiCDCClusterVersion{semver.New("5.2.0-master")}
c.Assert(ver.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(ver.ShouldEnableOldValueByDefault(), check.Equals, true)
require.Equal(t, ver.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, ver.ShouldEnableOldValueByDefault(), true)

c.Assert(TiCDCClusterVersionUnknown.ShouldEnableUnifiedSorterByDefault(), check.Equals, true)
c.Assert(TiCDCClusterVersionUnknown.ShouldEnableOldValueByDefault(), check.Equals, true)
require.Equal(t, TiCDCClusterVersionUnknown.ShouldEnableUnifiedSorterByDefault(), true)
require.Equal(t, TiCDCClusterVersionUnknown.ShouldEnableOldValueByDefault(), true)
}

func (s *checkSuite) TestCheckTiCDCClusterVersion(c *check.C) {
defer testleak.AfterTest(c)()

func TestCheckTiCDCClusterVersion(t *testing.T) {
t.Parallel()
testCases := []struct {
cdcClusterVersion TiCDCClusterVersion
expectedErr string
Expand Down Expand Up @@ -331,9 +320,9 @@ func (s *checkSuite) TestCheckTiCDCClusterVersion(c *check.C) {

for _, tc := range testCases {
isUnknown, err := CheckTiCDCClusterVersion(tc.cdcClusterVersion)
c.Assert(isUnknown, check.Equals, tc.expectedUnknown)
require.Equal(t, isUnknown, tc.expectedUnknown)
if len(tc.expectedErr) != 0 {
c.Assert(err, check.ErrorMatches, tc.expectedErr)
require.Regexp(t, tc.expectedErr, err)
}
}
}
24 changes: 24 additions & 0 deletions pkg/version/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2021 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package version

import (
"testing"

"github.com/pingcap/ticdc/pkg/leakutil"
)

func TestMain(m *testing.M) {
leakutil.SetUpLeakTest(m)
}