Skip to content

Comprehensive test#173

Merged
reyang merged 11 commits into
open-telemetry:masterfrom
ziqizh:comprehensive-test
Jul 21, 2020
Merged

Comprehensive test#173
reyang merged 11 commits into
open-telemetry:masterfrom
ziqizh:comprehensive-test

Conversation

@ziqizh
Copy link
Copy Markdown
Contributor

@ziqizh ziqizh commented Jul 15, 2020

Added a comprehensive ("integration") test case in `tracer_test.cc". I tried all possible combinations of samplers and tested its functionality in the tracer.

Comment thread sdk/test/trace/tracer_test.cc
Copy link
Copy Markdown

@jsandler18 jsandler18 left a comment

Choose a reason for hiding this comment

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

I am not sure if this qualifies as "integration tests". These seem more like comprehensive unit tests, which is great, but not exactly what you said.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 15, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@2c8b0e3). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #173   +/-   ##
=========================================
  Coverage          ?   95.56%           
=========================================
  Files             ?       91           
  Lines             ?     2548           
  Branches          ?        0           
=========================================
  Hits              ?     2435           
  Misses            ?      113           
  Partials          ?        0           
Impacted Files Coverage Δ
sdk/test/trace/tracer_test.cc 98.91% <100.00%> (ø)

@ziqizh
Copy link
Copy Markdown
Contributor Author

ziqizh commented Jul 15, 2020

I am not sure if this qualifies as "integration tests". These seem more like comprehensive unit tests, which is great, but not exactly what you said.

Yeah, we discussed in the SIG meeting and agreed to write a comprehensive unit test instead of an "integration" test.

Comment thread sdk/test/trace/tracer_test.cc Outdated
@ziqizh ziqizh marked this pull request as ready for review July 15, 2020 20:26
@ziqizh ziqizh requested a review from a team July 15, 2020 20:26
Copy link
Copy Markdown
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment thread sdk/test/trace/tracer_test.cc Outdated
Comment thread sdk/test/trace/tracer_test.cc Outdated
Comment thread sdk/test/trace/tracer_test.cc Outdated
@reyang reyang added the pr:please-merge This PR is ready to be merged by a Maintainer (rebased, CI passed, has enough valid approvals, etc.) label Jul 16, 2020
Comment thread sdk/test/trace/tracer_test.cc Outdated
Comment thread sdk/test/trace/tracer_test.cc Outdated
Comment thread sdk/test/trace/tracer_test.cc Outdated
@reyang reyang merged commit 49f6b9f into open-telemetry:master Jul 21, 2020
GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Jun 17, 2025
…s-create-or-update-comment-digest

Update peter-evans/create-or-update-comment digest to 362dbaf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:please-merge This PR is ready to be merged by a Maintainer (rebased, CI passed, has enough valid approvals, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants