Skip to content

Commit 536752a

Browse files
authored
ci: remove a bunch of flaky marks by fixing tests (#8044)
This change fixes or removes some tests that had been marked as "flaky": * Removes `test_tracer_safety.py`, which was skipped unconditionally in its entirety * Resolves a reliable failure in `test_logging.py` by telling the `DummyWriter` not to flush its traces to the test agent * Removes randomization from some `pytest_bdd` tests that expect their test suites to run in a specific order * Unskips a `snowflake` test that didn't fail in repeated local runs * Unskips a `test_utils_http` test that didn't fail in repeated local runs It's important to regularly revisit tests marked as `@flaky` and make sure they're reliable. `@flaky` is a temporary unblocking mechanism, not a permanent fix. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [ ] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [ ] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [ ] This PR doesn't touch any of that.
1 parent 721d301 commit 536752a

File tree

6 files changed

+12
-72
lines changed

6 files changed

+12
-72
lines changed

tests/contrib/asyncio/test_tracer_safety.py

Lines changed: 0 additions & 58 deletions
This file was deleted.

tests/contrib/logging/test_logging.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
from ddtrace.internal.constants import MAX_UINT_64BITS
1414
from ddtrace.vendor import wrapt
1515
from tests.utils import TracerTestCase
16-
from tests.utils import flaky
1716

1817

1918
logger = logging.getLogger()
@@ -198,8 +197,9 @@ def create_span():
198197
with self.override_global_config(dict(version="global.version", env="global.env")):
199198
self._test_logging(create_span=create_span, version="global.version", env="global.env")
200199

201-
@flaky(until=1704067200)
202-
@TracerTestCase.run_in_subprocess(env_overrides=dict(DD_TAGS="service:ddtagservice,env:ddenv,version:ddversion"))
200+
@TracerTestCase.run_in_subprocess(
201+
env_overrides=dict(DD_TAGS="service:ddtagservice,env:ddenv,version:ddversion", _DD_TEST_TRACE_FLUSH_ENABLED="0")
202+
)
203203
def test_log_DD_TAGS(self):
204204
def create_span():
205205
return self.tracer.trace("test.logging")

tests/contrib/pytest_bdd/test_pytest_bdd.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from tests.contrib.patch import emit_integration_and_version_to_test_agent
1515
from tests.utils import DummyCIVisibilityWriter
1616
from tests.utils import TracerTestCase
17-
from tests.utils import flaky
1817
from tests.utils import override_env
1918

2019

@@ -60,7 +59,6 @@ def test_and_emit_get_version(self):
6059

6160
emit_integration_and_version_to_test_agent("pytest-bdd", version)
6261

63-
@flaky(1707063167)
6462
def test_pytest_bdd_scenario_with_parameters(self):
6563
"""Test that pytest-bdd traces scenario with all steps."""
6664
self.testdir.makefile(
@@ -114,7 +112,7 @@ def check_cfparse(bars):
114112
"""
115113
)
116114
file_name = os.path.basename(py_file.strpath)
117-
self.inline_run("--ddtrace", file_name)
115+
self.inline_run("-p", "no:randomly", "--ddtrace", file_name)
118116
spans = self.pop_spans()
119117

120118
assert len(spans) == 13 # 3 scenarios + 7 steps + 1 module
@@ -156,7 +154,7 @@ def check():
156154
"""
157155
)
158156
file_name = os.path.basename(py_file.strpath)
159-
self.inline_run("--ddtrace", file_name)
157+
self.inline_run("-p", "no:randomly", "--ddtrace", file_name)
160158
spans = self.pop_spans()
161159

162160
assert len(spans) == 7
@@ -202,7 +200,7 @@ def check():
202200
"""
203201
)
204202
file_name = os.path.basename(py_file.strpath)
205-
self.inline_run("--ddtrace", file_name)
203+
self.inline_run("-p", "no:randomly", "--ddtrace", file_name)
206204
spans = self.pop_spans()
207205

208206
assert len(spans) == 7
@@ -225,7 +223,7 @@ def test_simple():
225223
"""
226224
)
227225
file_name = os.path.basename(py_file.strpath)
228-
self.inline_run("--ddtrace", file_name)
226+
self.inline_run("-p", "no:randomly", "--ddtrace", file_name)
229227
spans = self.pop_spans()
230228

231229
assert len(spans) == 4

tests/contrib/snowflake/test_snowflake.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from ddtrace.contrib.snowflake import patch
1212
from ddtrace.contrib.snowflake import unpatch
1313
from tests.opentracer.utils import init_tracer
14-
from tests.utils import flaky
1514
from tests.utils import override_config
1615
from tests.utils import snapshot
1716

@@ -176,7 +175,6 @@ def test_snowflake_service_env():
176175
assert cur.fetchone() == ("4.30.2",)
177176

178177

179-
@flaky(until=1704067200)
180178
@snapshot()
181179
@req_mock.activate
182180
def test_snowflake_pin_override(client):

tests/internal/test_utils_http.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@
22
import pytest
33

44
from ddtrace.internal.utils.http import connector
5-
from tests.utils import flaky
65

76

8-
@flaky(until=1704067201)
97
@pytest.mark.parametrize("scheme", ["http", "https"])
108
def test_connector(scheme):
119
with httpretty.enabled():

tests/utils.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from ddtrace.internal.encoding import JSONEncoder
3030
from ddtrace.internal.encoding import MsgpackEncoderV03 as Encoder
3131
from ddtrace.internal.schema import SCHEMA_VERSION
32+
from ddtrace.internal.utils.formats import asbool
3233
from ddtrace.internal.utils.formats import parse_tags_str
3334
from ddtrace.internal.writer import AgentWriter
3435
from ddtrace.propagation.http import _DatadogMultiHeader
@@ -570,6 +571,7 @@ class DummyTracer(Tracer):
570571

571572
def __init__(self, *args, **kwargs):
572573
super(DummyTracer, self).__init__()
574+
self._trace_flush_disabled_via_env = not asbool(os.getenv("_DD_TEST_TRACE_FLUSH_ENABLED", True))
573575
self._trace_flush_enabled = True
574576
self.configure(*args, **kwargs)
575577

@@ -610,7 +612,9 @@ def configure(self, *args, **kwargs):
610612
if not kwargs.get("writer"):
611613
# if no writer is present, check if test agent is running to determine if we
612614
# should emit traces.
613-
kwargs["writer"] = DummyWriter(trace_flush_enabled=check_test_agent_status())
615+
kwargs["writer"] = DummyWriter(
616+
trace_flush_enabled=check_test_agent_status() if not self._trace_flush_disabled_via_env else False
617+
)
614618
super(DummyTracer, self).configure(*args, **kwargs)
615619

616620

0 commit comments

Comments
 (0)