Using InMemorySpanExporter for wsgi/flask tests #306
Conversation
Signed-off-by: Alex Boten <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #306 +/- ##
==========================================
+ Coverage 84.51% 84.73% +0.21%
==========================================
Files 33 35 +2
Lines 1666 1716 +50
Branches 197 200 +3
==========================================
+ Hits 1408 1454 +46
- Misses 201 204 +3
- Partials 57 58 +1
Continue to review full report at Codecov.
|
| "http.status_code": 200, | ||
| "http.status_text": "OK", | ||
| } | ||
| expected_attrs = BoundedDict.from_map( |
There was a problem hiding this comment.
For expectations, I would not use BoundedDict because attributes could get dropped silently.
|
Merge conflicts have been resolved. Ready for reviews |
codeboten
added
the
needs reviewers
| spec_set=True, | ||
| return_value=self.span, | ||
| @classmethod | ||
| def setUpClass(cls): |
There was a problem hiding this comment.
We'll need to add a tearDownClass that undoes this (probably using importlib.reload). Otherwise, as soon as we have a second unit test that uses the same pattern, we'll get problems. See also the loader tests in the API package.
There was a problem hiding this comment.
Thanks for the pointer, I did not know about the reload method, will updated the test.
|
|
||
| self.memory_exporter = InMemorySpanExporter() | ||
| span_processor = export.SimpleExportSpanProcessor(self.memory_exporter) | ||
| tracer_source.add_span_processor(span_processor) |
There was a problem hiding this comment.
tracer_source would be shared by all test cases so you will be adding new span processor for every test case executed, maybe using a single InMemorySpanExporter and just call clear method during setup?
There was a problem hiding this comment.
Updated the test to set a single in-memory span exporter on class setup and clear it each time.
There was a problem hiding this comment.
I agree with @Oberon00 that there should be some way to undo the trace api tracer. But aside from that, looks great!
| @@ -49,100 +40,72 @@ def hello_endpoint(helloid): | |||
| self.client = Client(self.app, BaseResponse) | |||
|
|
|||
| def test_simple(self): | |||
| expected_attrs = { | |||
There was a problem hiding this comment.
some of these expected_attrs looks pretty common. could they be pooled into some common constructor or factory?
There was a problem hiding this comment.
updated to use a method with a dict argument for anything that the test needs to override
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
Fixes #303
This PR will have conflicts, I'll resolve them before moving this PR from draft.
Signed-off-by: Alex Boten [email protected]