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

testkit: add a WithTiKV flag to support run unit test on real TiKV #35647

Merged
merged 17 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
22 changes: 22 additions & 0 deletions testkit/mockstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,42 @@
package testkit

import (
"flag"
"testing"
"time"

"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/store/driver"
"github.com/pingcap/tidb/store/mockstore"
"github.com/stretchr/testify/require"
"github.com/tikv/client-go/v2/oracle"
"github.com/tikv/client-go/v2/tikv"
)

// WithTiKV flag makes the test case run with real TiKV
var WithTiKV = flag.String("with-tikv", "", "whether tests run with real TiKV")
Copy link
Member

Choose a reason for hiding this comment

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

In fact, @tisonkun has moved the real-tikv test case on here. If you would like to run the real-tikv test case. all are under the same folder.

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 background is, I need to test the paging protocol with the real tikv.
So I need rewrite all the test case or move them all?
A more reasonable solution is just add a flag ... if you don't use it, it doesn't change anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

This patch just add a flag, which won't affect CI settings. The reason I move all with real tikv tests to a dedicated test is for tagging tests in CI env.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tiancaiamao would you like to change CI logic? Or adding these code for conveniently running tests with real tikv locally?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, with-tikv is a string flag while its help info said "whether tests run with real TiKV", which is confusing...

Perhaps

// This flag is only used for debugging locally with real tikv cluster.
var tikvAddr = flag.String("tikv-addr", "", "address of tikv cluster, if set, running test with real tikv cluster")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, with-tikv is a string flag while its help info said "whether tests run with real TiKV", which is confusing...

Perhaps

// This flag is only used for debugging locally with real tikv cluster.
var tikvAddr = flag.String("tikv-addr", "", "address of tikv cluster, if set, running test with real tikv cluster")

Updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tiancaiamao would you like to change CI logic? Or adding these code for conveniently running tests with real tikv locally?

If you want to see the full changes, it's in my branch https://github.com/tiancaiamao/tidb/tree/test-with-tikv
But not all the test cases can run under TiKV, I check and document the result here:

https://pingcap.feishu.cn/docx/doxcnEzyRXsTdCJ2EE6EZwIP2oe

I'm not adding a new CI pipeline, I just make the code base more friendly for those who want to run some test cases with a real TiKV.


// CreateMockStore return a new mock kv.Storage.
func CreateMockStore(t testing.TB, opts ...mockstore.MockTiKVStoreOption) (store kv.Storage, clean func()) {
if *WithTiKV != "" {
var d driver.TiKVDriver
var err error
store, err = d.Open("tikv://" + *WithTiKV)
require.NoError(t, err)

var dom *domain.Domain
dom, err = session.BootstrapSession(store)
clean = func() {
dom.Close()
err := store.Close()
require.NoError(t, err)
}
require.NoError(t, err)
return
}

store, _, clean = CreateMockStoreAndDomain(t, opts...)
return
}
Expand Down
4 changes: 2 additions & 2 deletions tools/check/ut.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,9 +811,9 @@ func (n *numa) testCommand(pkg string, fn string) *exec.Cmd {
// it takes a longer when race is enabled. so it is set more timeout value.
args = append(args, []string{"-test.timeout", "30m"}...)
}
// session.test -test.run TestClusteredPrefixColum
args = append(args, "-test.run", fn)

// session.test -test.run TestClusteredPrefixColum
args = append(args, "-test.run", "^" + fn + "$")
return exec.Command(exe, args...)
}

Expand Down