-
Notifications
You must be signed in to change notification settings - Fork 455
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
[tests] Add support for resources.ExternalResources #3811
Conversation
1630bf0
to
10bd583
Compare
return err | ||
} | ||
|
||
p.resource = nil |
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.
why do we need to explicitly nil this out? Also do we need synchronization on this?
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.
This is more of a safety thing. If the resource is nil, we know that we've cleaned up properly or haven't started a previous run. The Setup
method explicitly checks for this.
Also, no synchronization should be needed here. There shouldn't be any concurrency on the same instance of a prometheus
object.
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.
LGTM
This commit adds an interface for adding external (i.e. not M3) resources that can be used in integration tests. An implementation of the interface that allows users to spin up a docker-backed Prometheus is added as well. This also functions as a working example for how the interface can be implemented to support other external resources.
10bd583
to
06fa74a
Compare
Codecov Report
@@ Coverage Diff @@
## master #3811 +/- ##
========================================
- Coverage 56.8% 56.8% -0.1%
========================================
Files 552 552
Lines 63079 63079
========================================
- Hits 35876 35834 -42
- Misses 24004 24038 +34
- Partials 3199 3207 +8
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
This commit adds an interface for adding external (i.e. not M3)
resources that can be used in integration tests. An
implementation of the interface that allows users to spin up
a docker-backed Prometheus is added as well. This also functions
as a working example for how the interface can be implemented to
support other external resources.
What this PR does / why we need it:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: