tests: ignore env var dependent tests at runtime #12805

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.

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>

components/tikv_alloc/src/jemalloc.rs

components/tikv_alloc/src/lib.rs

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

sticnarf

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

components/tikv_alloc/src/lib.rs

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

cmd/build.rs

@@ -32,7 +32,7 @@ fn link_sys_lib(lib: &str, tool: &cc::Tool) {
    }
    // 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

rust-toolchain

@@ -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.

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.

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.

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.

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 do-not-merge/needs-triage-completed size/M 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

components/tikv_alloc/src/jemalloc.rs Show resolved Hide resolved

components/tikv_alloc/src/lib.rs Show resolved Hide resolved

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 and removed size/M 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

components/tikv_alloc/src/lib.rs Outdated Show resolved Hide resolved

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 Status: PR - There is already 1 approval 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 Status: PR - There is already 1 approval label Jun 15, 2022

ti-chi-bot added the status/LGT2 Status: PR - There are already 2 approvals label Jun 15, 2022

Merge branch 'master' into optional-test

68b2dfa

ti-chi-bot added the status/can-merge Status: Can merge to base branch 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 Status: Can merge to base branch label Jun 15, 2022

ti-chi-bot added the status/can-merge Status: Can merge to base branch 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: ignore env var dependent tests at runtime #12805

tests: ignore env var dependent tests at runtime #12805

tabokie commented Jun 13, 2022 •

edited

ti-chi-bot commented Jun 13, 2022 •

edited

sticnarf left a comment

tabokie commented Jun 14, 2022 •

edited

Connor1996 Jun 14, 2022

tabokie Jun 14, 2022

BusyJay Jun 15, 2022

tabokie Jun 15, 2022 •

edited

sticnarf left a comment

BusyJay Jun 15, 2022

tabokie Jun 15, 2022

Connor1996 left a comment

tabokie commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

tabokie commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

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

tests: ignore env var dependent tests at runtime #12805

tests: ignore env var dependent tests at runtime #12805

Conversation

tabokie commented Jun 13, 2022 • edited

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot commented Jun 13, 2022 • edited

sticnarf left a comment

Choose a reason for hiding this comment

tabokie commented Jun 14, 2022 • edited

Connor1996 Jun 14, 2022

Choose a reason for hiding this comment

tabokie Jun 14, 2022

Choose a reason for hiding this comment

BusyJay Jun 15, 2022

Choose a reason for hiding this comment

tabokie Jun 15, 2022 • edited

Choose a reason for hiding this comment

sticnarf left a comment

Choose a reason for hiding this comment

BusyJay Jun 15, 2022

Choose a reason for hiding this comment

tabokie Jun 15, 2022

Choose a reason for hiding this comment

Connor1996 left a comment

Choose a reason for hiding this comment

tabokie commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

tabokie commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

ti-chi-bot commented Jun 15, 2022

tabokie commented Jun 13, 2022 •

edited

ti-chi-bot commented Jun 13, 2022 •

edited

tabokie commented Jun 14, 2022 •

edited

tabokie Jun 15, 2022 •

edited