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

Improving code coverage #939

Merged
merged 1 commit into from
Jul 29, 2020
Merged

Improving code coverage #939

merged 1 commit into from
Jul 29, 2020

Conversation

eddynaka
Copy link
Contributor

@eddynaka eddynaka commented Jul 28, 2020

Fixes #.

Changes

  • Removing unused methods/constructors
  • Adding more tests

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

  • Design discussion issue #
  • Changes in public API reviewed

@codecov
Copy link

codecov bot commented Jul 28, 2020

Codecov Report

Merging #939 into master will increase coverage by 1.64%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #939      +/-   ##
==========================================
+ Coverage   66.31%   67.96%   +1.64%     
==========================================
  Files         212      212              
  Lines        5994     5990       -4     
  Branches      967      967              
==========================================
+ Hits         3975     4071      +96     
+ Misses       1731     1641      -90     
+ Partials      288      278      -10     
Impacted Files Coverage Δ
...Exporter.ZPages/TracerProviderBuilderExtensions.cs 81.81% <ø> (ø)
...ry.Instrumentation.AspNet/AspNetInstrumentation.cs 88.88% <ø> (+16.16%) ⬆️
...umentation.AspNetCore/AspNetCoreInstrumentation.cs 83.33% <ø> (+20.83%) ⬆️
...ation.SqlClient/TracerProviderBuilderExtensions.cs 100.00% <ø> (+28.57%) ⬆️
src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs 80.00% <0.00%> (-13.34%) ⬇️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs 60.81% <0.00%> (-2.71%) ⬇️
...ngeRedis/StackExchangeRedisCallsInstrumentation.cs 76.36% <0.00%> (+3.63%) ⬆️
...ensions.Hosting/OpenTelemetryServicesExtensions.cs 66.66% <0.00%> (+13.33%) ⬆️
...Exporter.Jaeger/TracerProviderBuilderExtensions.cs 81.81% <0.00%> (+18.18%) ⬆️
... and 10 more

@eddynaka eddynaka marked this pull request as ready for review July 28, 2020 14:38
@eddynaka eddynaka requested a review from a team July 28, 2020 14:38
@eddynaka eddynaka changed the title [WIP] Improving code coverage Improving code coverage Jul 28, 2020
@alanwest
Copy link
Member

@eddynaka Looks like some nice improvements to test coverage. I'm generally interested in the approach you're taking to identify things to improve?

@eddynaka
Copy link
Contributor Author

@eddynaka Looks like some nice improvements to test coverage. I'm generally interested in the approach you're taking to identify things to improve?

Hi @alanwest , i'm using two approaches: vs2019 style and codecov report. From codecov report (https://codecov.io/gh/open-telemetry/opentelemetry-dotnet/tree/feature%2Fimproving-codecoverage/src), we can see which files have the lowest coverage. With that in mind, i started to do that.

@eddynaka
Copy link
Contributor Author

One thing that i still couldn't figure out is how to test EventSource. Injecting all the cases are hard, but at least we had to check the event and writeevent values.

Copy link
Member

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this @eddynaka!

@alanwest
Copy link
Member

Hi @alanwest , i'm using two approaches: vs2019 style and codecov report. From codecov report (https://codecov.io/gh/open-telemetry/opentelemetry-dotnet/tree/feature%2Fimproving-codecoverage/src), we can see which files have the lowest coverage. With that in mind, i started to do that.

Cool, thanks for the tip! I'd clicked around codecov a bit, and was just curious about your workflow.

@eddynaka
Copy link
Contributor Author

@alanwest, do u know how to test the event sources somehow?

Adding tests for zpages and hosting extensions

Testing enableConnectionLevelAttributes from SqlClient

updating assert

Adding tests for SqlClientInstrumentationOptions

removing extra test

undoing changes

updating changelog

renaming file to be complicant with other exporters

renaming files and adding more tests
@alanwest
Copy link
Member

@alanwest, do u know how to test the event sources somehow?

@eddynaka Sorry for delay, just seeing this...

Just to be sure, is it the SqlEventSourceTests tests you're looking at? I haven't spent a lot of time looking at this test suite, but I welcome the opportunity at familiarizing myself more! Can you point me to a specific test you'd like to improve?

@cijothomas cijothomas merged commit 3f01bdc into open-telemetry:master Jul 29, 2020
@eddynaka eddynaka deleted the feature/improving-codecoverage branch August 14, 2020 00:08
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.

4 participants