-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Feature/coverage azure pipelines #1704
Feature/coverage azure pipelines #1704
Conversation
It's a workaround, I will migrate to github action when it work well. |
vmImage: $(vmImage) | ||
|
||
# https://github.com/dutor/nebula-gears/issues/1 | ||
container: shylockhg/nebula-dev:centos7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use the image vesoft/nebula-dev:centos7
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the previous image, the newer compiler can't work well for coverage, the detail See the issue in comment.
add_compile_options(-g) | ||
add_compile_options(-O0) | ||
|
||
set(COVERAGES --coverage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this option valid for clang ? If not, please check the cmake compiler variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, valid for gcc/clang.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -0,0 +1,40 @@ | |||
# https://aka.ms/yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move this file into .github/workflows/?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It don't belong to github action.I think better put in root directory as other open source project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I don't sure will this make error for github action.
azure-pipelines.yml
Outdated
set -e | ||
cd _build | ||
lcov --capture --gcov-tool $GCOV --directory . --output-file coverage.info | ||
bash <(curl -s https://codecov.io/bash) -Z -f coverage.info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should remove system/third library. for example:
lcov --remove coverage.info '*/usr/include/*' '*/usr/lib/*' '*/usr/lib64/*' '*/usr/local/include/*' '*/usr/local/lib/*' '*/usr/local/lib64/*' '/opt/*' coverage.info -o final.info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
close #968 |
Delete https://github.com/vesoft-inc/nebula/tree/feature/coverage-azure-pipelines after merged. |
Change azure project 'Nebula Graph' to 'NebulaGraph' make the codecov could get project information by azure API. |
Duplicate with #1856 |
…two filter push-down rules. (vesoft-inc#1700)" (vesoft-inc#1704) This reverts commit b1f7f6f.
@dutor Need enable azure pipelines in github marketplace.
What changes were proposed in this pull request?
Add coverage collection and integrated with codecov.io
The azure pipeline not require token for codecov, so the PR from user without write permission work well too.
Why are the changes needed?
Testing coverage
Does this PR introduce any user-facing change?
NO
How was this patch tested?