telemetry: migrate test-infra to testify by tisonkun · Pull Request #26540 · pingcap/tidb

tisonkun · 2021-07-26T02:32:13Z

Signed-off-by: tison wander4096@gmail.com

What problem does this PR solve?

Issue Number: close #26537

What is changed and how it works?

ctetest for telemetry needs to run alone. Thus I write it a dedicate binary.
GetTxnUsageInfo and GetCTEUsageInfo need not to be public, align with other Get* functions.

Release note

No release note

Signed-off-by: tison <wander4096@gmail.com>

xhebox · 2021-07-26T03:27:13Z

telemetry/main_test.go

+)
+
+var (
+	GetFeatureUsage = getFeatureUsage


Maybe get a comment for these two, special keywords maybe. So that we can search it quickly later? I have a suggestion to have both tests with/without _test, to target the problem of access. But it is far from now.

I don't know what you mean well. Maybe show a patch?

like:

// FIXME: EXPORT_TRICK ... GetFeatureUsage = getFeatureUsage

My suggestion is to have tests splitted in two packages, one with the same package name as the code(no access problem), another is suffixed with _test(can only make use of exported functions). EXPORT_TRICK will be excellent when we want to search for all such tricks. Though it can also be archived by some scripts/source code parsing, we will look through all tests anyway.

For these two line, I tend to write the actual test under telemetry, but it failed with cyclic deps.

I get your point. You can create an issue to continue the discussion anyway. It provides a best practice for testing which may need a bit more time to see whether or not we really need it.

Yeah, for now, a mark is enough. Not really necessary though...

telemetry/data_cluster_hardware_test.go

telemetry/ctetest/cte_test.go

telemetry/telemetry_test.go

Signed-off-by: tison <wander4096@gmail.com>

telemetry/data_cluster_hardware_test.go

Signed-off-by: tison <wander4096@gmail.com>

ti-chi-bot · 2021-07-26T08:15:11Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

tiancaiamao
xhebox

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

tisonkun · 2021-07-27T09:37:47Z

/cc @mmyj

ti-chi-bot · 2021-07-27T09:37:48Z

@tisonkun: GitHub didn't allow me to request PR reviews from the following users: mmyj.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @mmyj

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/test-infra repository.

tiancaiamao · 2021-07-27T16:53:24Z

/merge

ti-chi-bot · 2021-07-27T16:53:26Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 304f945

ti-chi-bot · 2021-07-27T16:53:36Z

@tisonkun: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Details

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 ti-community-infra/tichi repository.

tiancaiamao · 2021-07-27T17:13:51Z

/run-check_dev_2

Still unstable?

[2021-07-27T17:02:23.621Z] The following test cases take too long to finish:
[2021-07-27T17:02:23.621Z] PASS: db_integration_test.go:1340: testIntegrationSuite6.TestCreateTableTooLarge	4.233s
[2021-07-27T17:02:23.621Z] PASS: db_partition_test.go:631: testIntegrationSuite1.TestCreateTableWithListPartition	4.741s
[2021-07-27T17:02:23.621Z] PASS: db_test.go:561: testDBSuite7.TestCancelAddIndex	4.871s
[2021-07-27T17:02:23.621Z] PASS: db_test.go:545: testDBSuite8.TestCancelAddPrimaryKey	4.279s
[2021-07-27T17:02:23.621Z] PASS: infoschema_reader_test.go:236: testInfoschemaTableSuite.TestDataTypesMaxLengthAndOctLength	4.059s
[2021-07-27T17:02:23.621Z] PASS: cte_test.go:244: CTETestSuite.TestCTEWithLimit	5.693s
[2021-07-27T17:02:23.621Z] PASS: join_test.go:1029: testSuiteJoin1.TestInSubquery	4.634s
[2021-07-27T17:02:23.621Z] PASS: partition_table_test.go:2954: testSuiteWithData.TestRangePartitionBoundariesBetweenM	4.753s
[2021-07-27T17:02:23.621Z] PASS: partition_table_test.go:749: partitionTableSuite.TestBatchGetforRangeandListPartitionTable	4.168s
[2021-07-27T17:02:23.621Z] PASS: analyze_test.go:594: testSuite1.TestAnalyzeIncremental	4.831s
[2021-07-27T17:02:23.621Z] PASS: analyze_test.go:169: testSuite1.TestAnalyzeParameters	4.328s
[2021-07-27T17:02:23.621Z] PASS: builtin_string_vec_test.go:521: testVectorizeSuite1.TestVectorizedBuiltinStringEvalOneVec	5.142s
[2021-07-27T17:02:23.621Z] PASS: builtin_time_vec_generated_test.go:6726: testVectorizeSuite1.TestVectorizedBuiltinTimeEvalOneVecGenerated	4.272s
[2021-07-27T17:02:23.621Z] exit status 252

telemetry: migrate test-infra to testify

3f1fb15

Signed-off-by: tison <wander4096@gmail.com>

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 26, 2021

tisonkun requested review from tiancaiamao and xhebox July 26, 2021 02:33

xhebox reviewed Jul 26, 2021

View reviewed changes

address comment

ffb9e81

Signed-off-by: tison <wander4096@gmail.com>

xhebox reviewed Jul 26, 2021

View reviewed changes

telemetry/data_cluster_hardware_test.go Outdated Show resolved Hide resolved

parallel from outer

304f945

Signed-off-by: tison <wander4096@gmail.com>

xhebox approved these changes Jul 26, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 26, 2021

tisonkun requested a review from tangenta July 27, 2021 02:54

tiancaiamao approved these changes Jul 27, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 27, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 27, 2021

Merge branch 'master' into issue-26537

7833ff4

ti-chi-bot merged commit 85cce04 into pingcap:master Jul 27, 2021

tisonkun deleted the issue-26537 branch July 27, 2021 23:00

sticnarf mentioned this pull request Jul 28, 2021

go.mod: update client-go to version 20210728 #26685

Merged

Conversation

tisonkun commented Jul 26, 2021

What problem does this PR solve?

What is changed and how it works?

Release note

Uh oh!

xhebox Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

tisonkun Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

xhebox Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

tisonkun Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

tisonkun Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

xhebox Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ti-chi-bot commented Jul 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tisonkun commented Jul 27, 2021

Uh oh!

ti-chi-bot commented Jul 27, 2021

Uh oh!

tiancaiamao commented Jul 27, 2021

Uh oh!

ti-chi-bot commented Jul 27, 2021

Uh oh!

ti-chi-bot commented Jul 27, 2021

Uh oh!

tiancaiamao commented Jul 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ti-chi-bot commented Jul 26, 2021 •

edited

Loading