-
Notifications
You must be signed in to change notification settings - Fork 824
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
feat: test-utils #644
feat: test-utils #644
Conversation
Codecov Report
@@ Coverage Diff @@
## master #644 +/- ##
==========================================
- Coverage 89.52% 89.52% -0.01%
==========================================
Files 188 188
Lines 9656 9691 +35
Branches 902 897 -5
==========================================
+ Hits 8645 8676 +31
- Misses 1011 1015 +4
|
edcdb4a
to
36449fc
Compare
36449fc
to
ecb41d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add fix
and check
npm scripts
✅ |
Would it make sense to include |
Other plugins such as dns and http have a different
But the function could potentially work for those as well provided it is configurable to the needs of those plugins. |
Yes definitely. |
Could you please do the same thing on pg-pool plugin? |
de72e45
to
200f499
Compare
2d1b82b
to
d6fde29
Compare
What's your plan on this? Are you planning to do it separate PR? |
I will include for pg, redis |
I was thinking to handle |
And use for redis and pg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks you!
Alright I think I will stop here with this PR, we can look into adding it to mysql,dns,http and other plugins in a separate PR. |
This is nice, thank you @naseemkullah :) |
My pleasure @mayurkale22 ! |
Which problem is this PR solving?
#617
Short description of the changes
A common
@opentelemetry/test-utils
to start and stop docker containers required for tests. The functions require adb
argument to indicate which container to start/stop. The allowed values areredis
,mysql
andpostgres
.Currently used in
redis
,mysql
andpostgres
plugins.If merged, it would have to be applied to
ioredis
(not yet merged) as well.P.S. Excuse the amends/force-pushes I presume nobody has looked at this yet and wanted a cleaner history.