-
Notifications
You must be signed in to change notification settings - Fork 535
Test Common w/ multiple OTel versions & add compat with old OTel #4312
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
base: potel-base
Are you sure you want to change the base?
Changes from all commits
0b59073
11462f2
46ef7c1
ee43828
e11e2ef
3de83dd
ca2185c
d1a365f
1282884
2085817
fa28dbf
03b8148
08d2982
c0fad24
16649eb
f74d3e1
e6d5808
b4b202f
19cb677
28256e2
f835a19
ebc2331
f10d404
e968394
819bd1f
27d525a
4b5c915
7bbc13e
bc03c6a
2f079d2
1c0dc4c
7937b41
4f8a369
752dad5
4ce2167
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,14 @@ | |
from concurrent import futures | ||
from textwrap import dedent | ||
from threading import Thread | ||
import sys | ||
|
||
import pytest | ||
|
||
import sentry_sdk | ||
from sentry_sdk import capture_message | ||
from sentry_sdk.integrations.threading import ThreadingIntegration | ||
from sentry_sdk.tracing import _OTEL_VERSION | ||
|
||
original_start = Thread.start | ||
original_run = Thread.run | ||
|
@@ -104,13 +106,18 @@ def double(number): | |
assert len(event["spans"]) == 0 | ||
|
||
|
||
@pytest.mark.skipif( | ||
sys.version[:3] == "3.8" and (1, 12) <= _OTEL_VERSION < (1, 16), | ||
reason="Fails in CI on 3.8 and specific OTel versions", | ||
) | ||
def test_circular_references(sentry_init, request): | ||
sentry_init(default_integrations=False, integrations=[ThreadingIntegration()]) | ||
|
||
gc.collect() | ||
gc.disable() | ||
request.addfinalizer(gc.enable) | ||
|
||
gc.collect() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moving this line is still necessary, even if we skip this test sometimes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving this makes the test suite pass locally on 3.8, so it's still an improvement though not in CI |
||
|
||
class MyThread(Thread): | ||
def run(self): | ||
pass | ||
|
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 unused now