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

Add thread sanitizer to test #72

Merged
merged 6 commits into from
Dec 20, 2021
Merged

Add thread sanitizer to test #72

merged 6 commits into from
Dec 20, 2021

Conversation

NobodyXu
Copy link
Contributor

This PR adds thread sanitizer to github workflow CI.

Signed-off-by: Jiahao XU Jiahao_XU@outlook.com

Copy link
Owner

@vorner vorner left a comment

Choose a reason for hiding this comment

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

Also, as mentioned in the linked ticket… I'm hesitant to merge it right now until we know if the reported failure is a false positive or not and until there's a solution. But once once that is solved, I'm happy to have it.

.github/workflows/test.yaml Outdated Show resolved Hide resolved
.github/workflows/test.yaml Outdated Show resolved Hide resolved
@vorner
Copy link
Owner

vorner commented Dec 19, 2021

Nice that the MacOS one works. But the UBuntu one would keep the CI red all the time, so it's not particularly useful.

Do you agree if I cherry-pick that one and leave the rest? Or do you want to create a cleaned-up branch with that one yourself?

@NobodyXu
Copy link
Contributor Author

Nice that the MacOS one works. But the UBuntu one would keep the CI red all the time, so it's not particularly useful.

Do you agree if I cherry-pick that one and leave the rest? Or do you want to create a cleaned-up branch with that one yourself?

I will remove the ubuntu thread sanitizer test since it is likely to be just false positive and really isn't helpful.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu
Copy link
Contributor Author

NobodyXu commented Dec 20, 2021

@vorner I've removed the Ubuntu thread sanitizer and rebased the branch again vorner:master.

All the workflows succeeded.

@codecov-commenter
Copy link

Codecov Report

Merging #72 (7c8398f) into master (f06b051) will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   86.25%   86.34%   +0.09%     
==========================================
  Files          18       18              
  Lines        1091     1091              
==========================================
+ Hits          941      942       +1     
+ Misses        150      149       -1     
Impacted Files Coverage Δ
src/strategy/hybrid.rs 93.84% <0.00%> (+1.53%) ⬆️

@vorner vorner merged commit d82b9e1 into vorner:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants