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

feat: support coverage ci for python sdk #2025

Merged
merged 5 commits into from
Jun 28, 2022
Merged

feat: support coverage ci for python sdk #2025

merged 5 commits into from
Jun 28, 2022

Conversation

mangoGoForward
Copy link
Contributor

Signed-off-by: mango [email protected]

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    feature

  • What is the current behavior? (You can also link to an open issue here)
    fixed test: add coverage report for python sdk #1971

  • What is the new behavior (if this is a feature change)?
    None

@github-actions github-actions bot added the workflow CICD related label Jun 23, 2022
@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #2025 (68a7b7e) into main (b78fb44) will increase coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #2025      +/-   ##
============================================
+ Coverage     75.60%   75.67%   +0.06%     
  Complexity      347      347              
============================================
  Files           612      615       +3     
  Lines        116926   116739     -187     
  Branches       1022     1015       -7     
============================================
- Hits          88404    88337      -67     
+ Misses        28313    28193     -120     
  Partials        209      209              
Impacted Files Coverage Δ
src/codec/field_codec.h 16.66% <0.00%> (-28.93%) ⬇️
src/rpc/rpc_client.h 85.13% <0.00%> (-4.06%) ⬇️
hybridse/src/udf/udf_test.h 88.67% <0.00%> (-3.78%) ⬇️
src/catalog/tablet_catalog.cc 70.77% <0.00%> (-3.00%) ⬇️
hybridse/src/codec/fe_row_codec.cc 78.49% <0.00%> (-0.32%) ⬇️
hybridse/src/codegen/udf_ir_builder.cc 82.62% <0.00%> (-0.31%) ⬇️
hybridse/include/codec/list_iterator_codec.h 80.85% <0.00%> (-0.21%) ⬇️
src/tablet/tablet_impl.cc 59.37% <0.00%> (-0.20%) ⬇️
...4paradigm/openmldb/batch/nodes/WindowAggPlan.scala 60.98% <0.00%> (-0.17%) ⬇️
hybridse/src/vm/runner.cc 67.77% <0.00%> (-0.04%) ⬇️
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b78fb44...68a7b7e. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2022

SDK Test Report

  75 files    75 suites   6m 7s ⏱️
163 tests 161 ✔️ 2 💤 0
204 runs  202 ✔️ 2 💤 0

Results for commit 68a7b7e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2022

Linux Test Report

       54 files       196 suites   49m 12s ⏱️
  8 993 tests   8 989 ✔️ 4 💤 0
13 239 runs  13 235 ✔️ 4 💤 0

Results for commit 68a7b7e.

♻️ This comment has been updated with latest results.

.github/workflows/sdk.yml Outdated Show resolved Hide resolved
.github/workflows/sdk.yml Outdated Show resolved Hide resolved
.github/workflows/sdk.yml Outdated Show resolved Hide resolved
@mangoGoForward
Copy link
Contributor Author

Hi @aceforeverd , the step of generate python coverage report failed, seems zookeeper can't connect. should we add a zookeeper container?

@aceforeverd
Copy link
Collaborator

Hi @aceforeverd , the step of generate python coverage report failed, seems zookeeper can't connect. should we add a zookeeper container?

Yes it dumps, should use steps/ut_zookeeper.sh to start zookeeper first, you can have a reference from the test sqlalchemy step, it have a script file

test -d /rambuild/ut_zookeeper && rm -rf /rambuild/ut_zookeeper/*
cp steps/zoo.cfg "$THIRDSRC/zookeeper-3.4.14/conf"

@mangoGoForward
Copy link
Contributor Author

should use steps/ut_zookeeper.sh to start zookeeper first, you can have a reference from the test sqlalchemy step, it have a script file

test -d /rambuild/ut_zookeeper && rm -rf /rambuild/ut_zookeeper/*
cp steps/zoo.cfg "$THIRDSRC/zookeeper-3.4.14/conf"

Thanks so much.

@aceforeverd
Copy link
Collaborator

aceforeverd commented Jun 24, 2022

should use steps/ut_zookeeper.sh to start zookeeper first, you can have a reference from the test sqlalchemy step, it have a script file

test -d /rambuild/ut_zookeeper && rm -rf /rambuild/ut_zookeeper/*
cp steps/zoo.cfg "$THIRDSRC/zookeeper-3.4.14/conf"

Thanks so much.

also test_python requires you to start the openmldb cluster, so I think you can just add a few coverage lines into test_python.sh for coverage

@mangoGoForward
Copy link
Contributor Author

Hi @aceforeverd , the CI of python-sdk has passed

@aceforeverd
Copy link
Collaborator

Hi @aceforeverd , the CI of python-sdk has passed

nice work. I see the coverage report but saidly it only show files in python/test, I guess that's because it is tested with the packaged openmdlb wheel, not from souce. we may further fix it

Copy link
Collaborator

@tobegit3hub tobegit3hub left a comment

Choose a reason for hiding this comment

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

LGTM

@tobegit3hub tobegit3hub merged commit b0aaea8 into 4paradigm:main Jun 28, 2022
@mangoGoForward mangoGoForward deleted the feat/support_python_coverage_ci branch June 28, 2022 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow CICD related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: add coverage report for python sdk
3 participants