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

[BUILD] move client::nosend under test_common #1811

Merged
merged 7 commits into from
Nov 28, 2022

Conversation

esigo
Copy link
Member

@esigo esigo commented Nov 26, 2022

Fixes #1226 (issue)

Changes

As client::nosend is meant to be used only for unit tests, this PR moves it under test_common. We could use this directory for any other common code needed for testing.

This directory is compiled only when BUILD_TESTING is set to ON.

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

@codecov
Copy link

codecov bot commented Nov 26, 2022

Codecov Report

Merging #1811 (ac148a8) into main (6a99fee) will decrease coverage by 0.02%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1811      +/-   ##
==========================================
- Coverage   85.79%   85.77%   -0.01%     
==========================================
  Files         171      171              
  Lines        5240     5240              
==========================================
- Hits         4495     4494       -1     
- Misses        745      746       +1     
Impacted Files Coverage Δ
sdk/src/trace/batch_span_processor.cc 90.63% <0.00%> (-0.78%) ⬇️

@esigo esigo marked this pull request as ready for review November 26, 2022 18:50
@esigo esigo requested a review from a team as a code owner November 26, 2022 18:50
@esigo esigo changed the title move client::nosend under test_common [TEST] move client::nosend under test_common Nov 26, 2022
@esigo esigo changed the title [TEST] move client::nosend under test_common [BUILD] move client::nosend under test_common Nov 27, 2022
Copy link
Member

@owent owent left a comment

Choose a reason for hiding this comment

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

LGTM

@esigo esigo merged commit ec90007 into open-telemetry:main Nov 28, 2022
@esigo esigo deleted the 1226-nosend branch November 28, 2022 16:03
@lalitb
Copy link
Member

lalitb commented Nov 28, 2022

How about moving this to exporters/test_common or ext/test_common, just to keep the root of repo clean, with only required directories ?

yxue pushed a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove gmock/gtest depepdency from ext::http::client::nosend
3 participants