tests: ignore env var dependent tests at runtime by tabokie · Pull Request #12805 · tikv/tikv

tabokie · 2022-06-13T05:53:40Z

Signed-off-by: tabokie xy.tao@outlook.com

What is changed and how it works?

Issue Number: Close #12804

What's Changed:

Implement rust-lang/rust#68007 using custom test runner.

The test runner is deliberately not placed in test_util crate to avoid dependency.

Also bump rust-toolchain to 2022-05-01 to use ignore message (rust-lang/cargo#10250, rust-lang/rust#96132). We can implement similar functionality by parsing test name, but that will be slower and not very UI friendly.

The only reason I still keep two lines in test script is to avoid repeated <jemalloc>: Invalid conf pair: prof:true yelling during build.

Related changes

Check List

Tests

Unit test

Manual test:

# cargo test --features mem-profiling --features jemalloc -p tikv_alloc ifdef_malloc_conf
test imp::profiling::tests::test_profiling_memory_ifdef_malloc_conf ... ignored, #ifdef MALLOC_CONF
# MALLOC_CONF=prof:true cargo test --features mem-profiling --features jemalloc -p tikv_alloc ifdef_malloc_conf
test imp::profiling::tests::test_profiling_memory_ifdef_malloc_conf ... ok

Release note

None

Signed-off-by: tabokie <xy.tao@outlook.com>

ti-chi-bot · 2022-06-13T05:53:42Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Connor1996
sticnarf

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.

Signed-off-by: tabokie <xy.tao@outlook.com>

sticnarf

Do you know why CI fails because tikv-ctl depends on libstdc++?

tabokie · 2022-06-14T07:32:52Z

Do you know why CI fails because tikv-ctl depends on libstdc++?

~~I do not. Upstream reports rust-lang/rust#94983 but it didn't fix our issue. I'm trying to reproduce it in a smaller repo and bisect the cause.~~

The regression is caused by rust-lang/rust#93901. Fixed by manually adding that flag back.

Signed-off-by: tabokie <xy.tao@outlook.com>

Connor1996 · 2022-06-14T10:14:06Z

    // remove lib prefix and .a postfix.
    let libname = &lib[3..lib.len() - 2];
-    println!("cargo:rustc-link-lib=static={}", &libname);
+    println!("cargo:rustc-link-lib=static:+whole-archive={}", &libname);


what's this

From what I can read, whole-archive flag will force using static symbols when there're both a static and a dynamic version available. It is enabled by default before. The fact that we must enable it to pass the check means we somehow linked multiple versions of libstdc++.

How can that happen? I think it should report duplicated symbol error.

I meant to say that multiple versions are available for the linker. So it's up to the implementation to choose one of them. ~~And FYI during the debugging I have ruled out rust-rocksdb by setting the static_libcpp feature.~~

sticnarf

LGTM

BusyJay · 2022-06-15T06:37:05Z

@@ -1 +1 @@
-nightly-2022-02-14
+nightly-2022-05-01


Why use 05-01?

rust-lang/rust#96132 is fixed in 04-17.

Connor1996

LGTM

tabokie · 2022-06-15T11:36:10Z

/merge

ti-chi-bot · 2022-06-15T11:36:11Z

@tabokie: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and 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.

If you have any questions about the PR merge process, please refer to pr process.

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.

ti-chi-bot · 2022-06-15T11:36:13Z

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

Details

Commit hash: 68b2dfa

Signed-off-by: tabokie <xy.tao@outlook.com>

tabokie · 2022-06-15T11:56:18Z

/merge

ti-chi-bot · 2022-06-15T11:56:19Z

@tabokie: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and 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.

If you have any questions about the PR merge process, please refer to pr process.

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.

ti-chi-bot · 2022-06-15T11:56:21Z

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

Details

Commit hash: 39d35f3

ignore env var dependent tests at runtime

8cb6e30

Signed-off-by: tabokie <xy.tao@outlook.com>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 13, 2022

clean up

67df62a

Signed-off-by: tabokie <xy.tao@outlook.com>

tabokie requested review from BusyJay and Connor1996 June 13, 2022 05:57

Connor1996 reviewed Jun 13, 2022

View reviewed changes

Comment thread components/tikv_alloc/src/jemalloc.rs

Comment thread components/tikv_alloc/src/lib.rs

ti-chi-bot removed the do-not-merge/needs-triage-completed label Jun 13, 2022

tabokie force-pushed the optional-test branch from e05ad7e to 259fbd2 Compare June 13, 2022 09:18

fix test script and address comment

355b252

Signed-off-by: tabokie <xy.tao@outlook.com>

tabokie force-pushed the optional-test branch from 259fbd2 to 355b252 Compare June 13, 2022 09:20

tabokie added 2 commits June 13, 2022 17:33

clippy

1ddb4da

Signed-off-by: tabokie <xy.tao@outlook.com>

use ignore message instead of test name

1bb8267

Signed-off-by: tabokie <xy.tao@outlook.com>

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 13, 2022

update packages and fix lints

7d73256

Signed-off-by: tabokie <xy.tao@outlook.com>

tabokie force-pushed the optional-test branch from d4703e4 to 7d73256 Compare June 13, 2022 12:12

BusyJay requested a review from sticnarf June 13, 2022 17:37

sticnarf reviewed Jun 14, 2022

View reviewed changes

Comment thread components/tikv_alloc/src/lib.rs Outdated

tabokie added 2 commits June 14, 2022 18:08

force whole archive flag

af700c2

Signed-off-by: tabokie <xy.tao@outlook.com>

address comment

4d6dc57

Signed-off-by: tabokie <xy.tao@outlook.com>

Connor1996 reviewed Jun 14, 2022

View reviewed changes

sticnarf approved these changes Jun 15, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 15, 2022

BusyJay reviewed Jun 15, 2022

View reviewed changes

Connor1996 approved these changes Jun 15, 2022

View reviewed changes

ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jun 15, 2022

ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Jun 15, 2022

Merge branch 'master' into optional-test

68b2dfa

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 15, 2022

fix error from merge

39d35f3

Signed-off-by: tabokie <xy.tao@outlook.com>

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 15, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 15, 2022

ti-chi-bot merged commit 2fbf7ee into tikv:master Jun 15, 2022

tabokie deleted the optional-test branch June 15, 2022 12:29

tabokie mentioned this pull request Jun 17, 2022

tests: natively support nextest #12799

Merged

		@@ -1 +1 @@
		nightly-2022-02-14
		nightly-2022-05-01

Conversation

tabokie commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sticnarf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tabokie commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Connor1996 Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

tabokie Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

BusyJay Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

tabokie Jun 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sticnarf left a comment

Choose a reason for hiding this comment

Uh oh!

BusyJay Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

tabokie Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

Connor1996 left a comment

Choose a reason for hiding this comment

Uh oh!

tabokie commented Jun 15, 2022

Uh oh!

ti-chi-bot commented Jun 15, 2022

Uh oh!

ti-chi-bot commented Jun 15, 2022

Uh oh!

tabokie commented Jun 15, 2022

Uh oh!

ti-chi-bot commented Jun 15, 2022

Uh oh!

ti-chi-bot commented Jun 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tabokie commented Jun 13, 2022 •

edited

Loading

ti-chi-bot commented Jun 13, 2022 •

edited

Loading

tabokie commented Jun 14, 2022 •

edited

Loading

tabokie Jun 15, 2022 •

edited

Loading