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.

Sign up for GitHub

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

Jump to bottom

Increase Test Coverage for XML Http Plugin #1216

Merged
merged 12 commits into from
Jun 23, 2020
Merged

Increase Test Coverage for XML Http Plugin #1216

merged 12 commits into from
Jun 23, 2020

Conversation

thgao
Copy link
Contributor

@thgao thgao commented Jun 18, 2020

Which problem is this PR solving?

Short description of the changes

  • Added tests to increase code coverage in xhr.ts to at least 90%
  • note: opentelemetry-web already has it's code coverage increased to >90% now

thgao added 10 commits June 18, 2020 15:13
@thgao
test(opentelemetry-plugin-xml-http-request): added un/patching tests
d80f37f 
Signed-off-by: Tina Gao <thgao@uwaterloo.ca>
@thgao
test(opentelemetry-plugin-xml-http-request): added CORS preflight tests
c7f8d33
@thgao
style(opentelemetry-plugin-xml-http-request): ran lint:fix
595ee85
@thgao
test(opentelemetry-plugin-xml-http-request): ignored urls
0898236 
added test for when request url is an ignored url from plugin config
@thgao
test(opentelemetry-plugin-xml-http-request): header prop tests
7a88ba5 
fix header propagation tests that were affected by added a preflight
span
@thgao
test(opentelemetry-plugin-xml-http-request): clearTimingResources config
5d8c2c6 
Added tests for clearTimingResources setting in the plugin's config for
the default behaviour when unspecified (false) and for explicitly
setting to true.
@thgao
test(opentelemetry-plugin-xml-http-request): added test for XHR reuse
b9fc81a
@thgao
test(opentelemetry-plugin-xml-http-request): added error handler tests
8f11a63
@thgao
style(opentelemetry-plugin-xml-http-request): cleaned up
b0fedd7
@thgao
refactor(opentelemetry-plugin-xml-http-request): use common getData
0ca3f9f
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 18, 2020 •
edited
Loading

CLA Check
The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Jun 18, 2020 •
edited
Loading

Codecov Report

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

@@            Coverage Diff             @@
##           master    #1216      +/-   ##
==========================================
+ Coverage   92.32%   93.20%   +0.87%     
==========================================
  Files         122      122              
  Lines        3533     3533              
  Branches      714      714              
==========================================
+ Hits         3262     3293      +31     
+ Misses        271      240      -31
Impacted Files Coverage Δ
...s/opentelemetry-plugin-xml-http-request/src/xhr.ts 93.88% <0.00%> (+17.22%) ⬆️

@dyladan dyladan mentioned this pull request Jun 18, 2020
Merged
mayurkale22
mayurkale22 approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

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

This is nice, thanks for working on this.

@mayurkale22
Copy link
Member

@open-telemetry/javascript-approvers Please take a look when you get a chance.

dyladan
dyladan approved these changes Jun 22, 2020
View reviewed changes
Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

Test coverage is something that gets left behind too often. Nice to have some improvements here.

@mayurkale22 mayurkale22 added enhancement New feature or request Merge:LGTM This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) labels Jun 22, 2020
@mayurkale22 mayurkale22 merged commit 33a1297 into open-telemetry:master Jun 23, 2020
@mayurkale22 mayurkale22 mentioned this pull request Jun 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naseemkullah naseemkullah naseemkullah approved these changes

@mayurkale22 mayurkale22 mayurkale22 approved these changes

@markwolff markwolff markwolff approved these changes

@dyladan dyladan dyladan approved these changes

@obecny obecny obecny approved these changes

@bg451 bg451 Awaiting requested review from bg451

@legendecas legendecas Awaiting requested review from legendecas

@mwear mwear Awaiting requested review from mwear

@OlivierAlbertini OlivierAlbertini Awaiting requested review from OlivierAlbertini

@rochdev rochdev Awaiting requested review from rochdev

@vmarchaud vmarchaud Awaiting requested review from vmarchaud

Assignees
No one assigned
Labels
enhancement New feature or request Merge:LGTM This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@thgao @mayurkale22 @obecny @dyladan @markwolff @naseemkullah