Skip to content

Add unit-test for Jaeger exporter#1151

Merged
ThomsonTan merged 10 commits into
open-telemetry:mainfrom
esigo:Add-unit-test-for-Jaeger-exporter
Dec 21, 2021
Merged

Add unit-test for Jaeger exporter#1151
ThomsonTan merged 10 commits into
open-telemetry:mainfrom
esigo:Add-unit-test-for-Jaeger-exporter

Conversation

@esigo
Copy link
Copy Markdown
Member

@esigo esigo commented Dec 18, 2021

Fixes #1142 (issue)

Changes

Add unit-test for Jaeger exporter

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@esigo esigo requested a review from a team December 18, 2021 23:39
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 18, 2021

Codecov Report

Merging #1151 (4944362) into main (4803310) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1151   +/-   ##
=======================================
  Coverage   93.39%   93.39%           
=======================================
  Files         165      165           
  Lines        6230     6230           
=======================================
  Hits         5818     5818           
  Misses        412      412           

Comment thread exporters/jaeger/CMakeLists.txt Outdated

add_executable(jaeger_exporter_test test/jaeger_exporter_test.cc)
add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1)
if(GMOCK_LIB)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Put this check under MSVC as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks, done

using namespace jaegertracing;
using namespace opentelemetry::exporter::jaeger;
using namespace opentelemetry::sdk::instrumentationlibrary;
using std::vector;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks, removed


TEST_F(JaegerExporterTestPeer, ShutdownTest)
{
auto mock_thrift_ender = new MockThriftSender;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit; would mock_thrift_sender be better naming?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks, it was a typo.

@ThomsonTan ThomsonTan merged commit 9a6c745 into open-telemetry:main Dec 21, 2021
@esigo esigo deleted the Add-unit-test-for-Jaeger-exporter branch December 21, 2021 06:44
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.

Add unit-test for Jaeger exporter

3 participants