Skip to content

Commit d286964

Browse files
authored
Check that declared version are not ahead of tested version in dev mode (#6774)
1 parent 568ddaa commit d286964

9 files changed

Lines changed: 224 additions & 17 deletions

File tree

.github/workflows/run-end-to-end.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ on:
101101
default: "datadoghq.com"
102102
required: false
103103
type: string
104+
_system_tests_dev_mode:
105+
description: "Shall we run system tests in dev mode (library and agent dev binary)"
106+
default: false
107+
required: false
108+
type: boolean
104109

105110
jobs:
106111
main:
@@ -111,6 +116,7 @@ jobs:
111116
SYSTEM_TESTS_REPORT_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
112117
SYSTEM_TESTS_SKIP_EMPTY_SCENARIO: ${{ inputs.skip_empty_scenarios }}
113118
SYSTEM_TESTS_FORCE_EXECUTE: ${{ inputs.force_execute }}
119+
SYSTEM_TESTS_DEV_MODE: ${{ inputs._system_tests_dev_mode }}
114120
steps:
115121
- name: Compute ref
116122
id: compute_ref

.github/workflows/system-tests.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ jobs:
287287
_build_proxy_image: ${{ inputs._build_proxy_image }}
288288
_build_lambda_proxy_image: ${{ inputs._build_lambda_proxy_image }}
289289
_enable_replay_scenarios: ${{ inputs._enable_replay_scenarios }}
290+
_system_tests_dev_mode: ${{ inputs._system_tests_dev_mode }}
290291

291292
display_summary:
292293
name: Report failures

conftest.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
from utils import context
2222
from utils._context._scenarios import Scenario, scenarios
23-
from utils._context.component_version import ComponentVersion, Version
23+
from utils._context.component_version import ComponentVersion
2424
from utils.const import COMPONENT_GROUPS
2525
from utils._decorators import add_pytest_marker
2626
from utils._decorators import configure as configure_decorators
@@ -233,7 +233,7 @@ def pytest_sessionstart(session: pytest.Session) -> None:
233233
if not session.config.option.collectonly:
234234
context.scenario.pytest_sessionstart(session)
235235

236-
# The canonical way o adding Junit properties to testsuite is not working with xdist
236+
# The canonical way of adding Junit properties to testsuite is not working with xdist
237237
# Workaround to tackle this issue
238238
# https://github.com/pytest-dev/pytest/issues/7767#issuecomment-698560400
239239
xml = session.config._store.get(xml_key, None) # noqa: SLF001
@@ -247,6 +247,15 @@ def pytest_sessionstart(session: pytest.Session) -> None:
247247
logger.terminal.write(" *** .:: Sleep mode activated. Press Ctrl+C to exit ::. *** ")
248248
logger.terminal.write("\n ********************************************************** \n\n")
249249

250+
if os.environ.get("SYSTEM_TESTS_DEV_MODE", "").lower() == "true":
251+
logger.info("Checking that no version is ahead of main branch")
252+
manifest = Manifest(context.scenario.components, context.weblog_variant)
253+
errors = manifest.assert_versions_not_ahead_of_current()
254+
if errors:
255+
message = "Dev mode check: manifest declares versions ahead of the current tested version:\n"
256+
message += "\n".join(f" - {e}" for e in errors)
257+
pytest.exit(message, 1)
258+
250259

251260
# called when each test item is collected
252261
def _collect_item_metadata(item: pytest.Item):
@@ -290,10 +299,8 @@ def pytest_collection_modifyitems(session: pytest.Session, config: pytest.Config
290299
"""Unselect items that were deactivated in the manifests or that are not included in the current scenario"""
291300

292301
logger.debug("pytest_collection_modifyitems")
293-
manifest_components: dict[str, Version] = {
294-
name: version for name, version in context.scenario.components.items() if isinstance(version, Version)
295-
}
296-
manifest = Manifest(manifest_components, context.weblog_variant)
302+
manifest = Manifest(context.scenario.components, context.weblog_variant)
303+
297304
for item in items:
298305
assert isinstance(item, pytest.Function)
299306
declarations = manifest.get_declarations(item.nodeid)
@@ -399,6 +406,8 @@ def _item_is_skipped(item: pytest.Item):
399406

400407

401408
def pytest_collection_finish(session: pytest.Session) -> None:
409+
logger.debug("pytest_collection_finish")
410+
402411
if session.config.option.collectonly:
403412
return
404413

manifests/python.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1951,7 +1951,7 @@ manifest:
19511951
tests/parametric/test_trace_sampling.py::Test_Trace_Sampling_Tags_Feb2024_Revision::test_metric_mismatch_non_integer: bug (APMAPI-1689)
19521952
tests/parametric/test_trace_sampling.py::Test_Trace_Sampling_With_W3C: v2.8.0
19531953
tests/parametric/test_tracer.py::Test_ProcessTags_ServiceName: v4.7.0
1954-
tests/parametric/test_tracer.py::Test_ProcessTags_ServiceName::test_process_tag_svc_auto: v4.8.0
1954+
tests/parametric/test_tracer.py::Test_ProcessTags_ServiceName::test_process_tag_svc_auto: v4.8.0-dev
19551955
tests/parametric/test_tracer.py::Test_Tracer: v2.8.0
19561956
tests/parametric/test_tracer.py::Test_TracerSCITagging: v1.12.0
19571957
tests/parametric/test_tracer.py::Test_TracerServiceNameSource: irrelevant

manifests/ruby.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1572,7 +1572,7 @@ manifest:
15721572
tests/parametric/test_partial_flushing.py::Test_Partial_Flushing::test_partial_flushing_one_span_default: missing_feature # Created by easy win activation script
15731573
tests/parametric/test_process_discovery.py::Test_ProcessDiscovery: v2.18.0
15741574
tests/parametric/test_sampling_delegation.py::Test_Decisionless_Extraction: v2.4.0
1575-
tests/parametric/test_sampling_span_tags.py::Test_Knuth_Sample_Rate: '>=2.31.0'
1575+
tests/parametric/test_sampling_span_tags.py::Test_Knuth_Sample_Rate: v2.31.0-dev
15761576
tests/parametric/test_sampling_span_tags.py::Test_Sampling_Span_Tags::test_tags_appsec_enabled_sst011: bug (APMAPI-737)
15771577
tests/parametric/test_sampling_span_tags.py::Test_Sampling_Span_Tags::test_tags_child_dropped_sst001: bug (APMAPI-737)
15781578
tests/parametric/test_sampling_span_tags.py::Test_Sampling_Span_Tags::test_tags_child_kept_sst007: bug (APMAPI-737)

tests/test_the_test/test_manifest.py

Lines changed: 134 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,25 @@
66
from utils import scenarios
77
from utils._context.component_version import Version
88
from utils.manifest import Manifest, SkipDeclaration, TestDeclaration
9-
from utils.manifest._internal.types import SemverRange as CustomSpec
9+
from utils.manifest._internal.types import ManifestData, SemverRange as CustomSpec, Condition
1010
from utils.manifest._internal.validate import assert_nodeids_exist
1111
from utils.scripts.activate_easy_wins._internal.manifest_editor import ManifestEditor
1212
from utils.scripts.activate_easy_wins._internal.types import Context
1313

1414

1515
def manifest_init(
16-
components: dict[str, Version],
16+
components: dict[str, Version | str],
1717
weblog: str = "some_variant",
1818
path: Path = Path("tests/test_the_test/manifests/manifests_parser_test/"),
19-
):
20-
return Manifest(components, weblog, path)
19+
manifest_data: dict[str, list[Condition]] | None = None,
20+
) -> Manifest:
21+
result = Manifest(components, weblog, path)
22+
if manifest_data:
23+
result.data = ManifestData()
24+
for key, value in manifest_data.items():
25+
result.data[key] = value
26+
27+
return result
2128

2229

2330
@scenarios.test_the_test
@@ -403,3 +410,126 @@ def test_selects_correct_component_when_converting_inline_to_list(self):
403410
first_entry = python_manifest_rule[0]
404411
assert first_entry == {"declaration": "bug (TICKET-123)"}
405412
assert "excluded_component_version" not in first_entry
413+
414+
415+
def _get_manifest_errors(component: str, *, declared_version: str, components: dict[str, Version | str]) -> list[str]:
416+
manifest = manifest_init(
417+
components,
418+
manifest_data={
419+
"tests/foo.py::TestFoo": [
420+
{
421+
"component": component,
422+
"component_version": CustomSpec(declared_version),
423+
"excluded_component_version": CustomSpec(declared_version),
424+
"declaration": SkipDeclaration("missing_feature"),
425+
}
426+
]
427+
},
428+
)
429+
430+
return manifest.assert_versions_not_ahead_of_current()
431+
432+
433+
@scenarios.test_the_test
434+
class Test_VersionsNotAheadOfCurrent:
435+
def test_declared_version_below_current_is_ok(self):
436+
"""A declared version lower than the current version produces no errors."""
437+
errors = _get_manifest_errors(
438+
"nodejs",
439+
declared_version=">=5.0.0",
440+
components={"nodejs": Version("5.2.0")},
441+
)
442+
assert errors == []
443+
444+
def test_declared_version_equals_current_is_ok(self):
445+
"""A declared version equal to the current version produces no errors."""
446+
errors = _get_manifest_errors(
447+
"nodejs",
448+
declared_version=">=5.2.0",
449+
components={"nodejs": Version("5.2.0")},
450+
)
451+
assert errors == []
452+
453+
def test_declared_version_with_prerelease(self):
454+
"""Declaring v5.2.0 while testing 5.2.0-dev is not allowed."""
455+
errors = _get_manifest_errors(
456+
"nodejs",
457+
declared_version=">=5.2.0",
458+
components={"nodejs": Version("5.2.0-dev")},
459+
)
460+
assert len(errors) == 2
461+
462+
def test_declared_version_above_current_is_an_error(self):
463+
"""A declared version higher than the current version produces an error per field."""
464+
errors = _get_manifest_errors(
465+
"nodejs",
466+
declared_version=">=6.0.0",
467+
components={"nodejs": Version("5.2.0-dev")},
468+
)
469+
assert len(errors) == 2
470+
assert all("nodejs" in e for e in errors)
471+
assert all("6.0.0" in e for e in errors)
472+
assert all("5.2.0" in e for e in errors)
473+
474+
def test_caret_notation_above_current_is_an_error(self):
475+
"""^X.Y.Z is treated as a lower bound; flagged for each field when it exceeds current version."""
476+
errors = _get_manifest_errors(
477+
"nodejs",
478+
declared_version="^6.0.0",
479+
components={"nodejs": Version("5.2.0")},
480+
)
481+
assert len(errors) == 2
482+
483+
def test_or_expression_is_treated(self):
484+
"""Multi-branch OR expressions are not ignored."""
485+
errors = _get_manifest_errors(
486+
"nodejs",
487+
declared_version=">=6.0.0 || ^3.0.0",
488+
components={"nodejs": Version("5.2.0-dev")},
489+
)
490+
assert len(errors) == 2
491+
492+
def test_no_excluded_component_version_is_ok(self):
493+
"""Conditions without excluded_component_version (plain skip declarations) are ignored."""
494+
manifest = manifest_init(
495+
components={"nodejs": Version("5.2.0")},
496+
manifest_data={
497+
"tests/foo.py::TestFoo": [{"component": "nodejs", "declaration": SkipDeclaration("missing_feature")}]
498+
},
499+
)
500+
501+
errors = manifest.assert_versions_not_ahead_of_current()
502+
assert errors == []
503+
504+
def test_unknown_component_is_skipped(self):
505+
"""Conditions for components not in the tested set are ignored."""
506+
errors = _get_manifest_errors(
507+
"java",
508+
declared_version=">=6.0.0",
509+
components={"nodejs": Version("5.2.0")},
510+
)
511+
assert errors == []
512+
513+
def test_multiple_violations_are_all_reported(self):
514+
"""Every offending condition is reported, not just the first."""
515+
manifest = manifest_init(
516+
components={"nodejs": Version("5.2.0")},
517+
manifest_data={
518+
"tests/foo.py::TestFoo": [
519+
{
520+
"component": "nodejs",
521+
"excluded_component_version": CustomSpec(">=6.0.0"),
522+
"declaration": SkipDeclaration("missing_feature"),
523+
}
524+
],
525+
"tests/bar.py::TestBar": [
526+
{
527+
"component": "nodejs",
528+
"component_version": CustomSpec(">=7.0.0"),
529+
"declaration": SkipDeclaration("missing_feature"),
530+
}
531+
],
532+
},
533+
)
534+
errors = manifest.assert_versions_not_ahead_of_current()
535+
assert len(errors) == 2

utils/manifest/__init__.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
11
from ._internal import Manifest
22
from ._internal import SkipDeclaration, Condition, ManifestData, TestDeclaration
33

4-
__all__ = ["Condition", "Manifest", "ManifestData", "SkipDeclaration", "TestDeclaration"]
4+
__all__ = [
5+
"Condition",
6+
"Manifest",
7+
"ManifestData",
8+
"SkipDeclaration",
9+
"TestDeclaration",
10+
]

utils/manifest/_internal/__init__.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,10 @@
22
from .core import Manifest
33
from .types import SkipDeclaration, Condition, ManifestData
44

5-
__all__ = ["Condition", "Manifest", "ManifestData", "SkipDeclaration", "TestDeclaration"]
5+
__all__ = [
6+
"Condition",
7+
"Manifest",
8+
"ManifestData",
9+
"SkipDeclaration",
10+
"TestDeclaration",
11+
]

utils/manifest/_internal/core.py

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
from pathlib import Path
2-
from .parser import load
2+
import re
3+
34
from utils._context.component_version import Version
5+
6+
from .parser import load
47
from .rule import get_rules, match_rule
58
from .types import ManifestData, SkipDeclaration
69
from .validate import validate_manifest_files as validate
710
from .const import default_manifests_path
811
from .format import yml_sort
912

1013

14+
_LOWER_BOUND_RE = re.compile(r"(?:>=?|\^)(\d+\.\d+(?:\.\d+)?[.\w+-]*)")
15+
16+
1117
class Manifest:
1218
"""Provides a simple way to get information from the manifests"""
1319

@@ -17,7 +23,7 @@ class Manifest:
1723

1824
def __init__(
1925
self,
20-
components: dict[str, Version] | None = None,
26+
components: dict[str, Version | str] | None = None, # TODO : remove str versions from scenario.components
2127
weblog: str | None = None,
2228
path: Path = default_manifests_path,
2329
):
@@ -27,7 +33,10 @@ def __init__(
2733
self.data = load(path)
2834
self.rules = None
2935
if components is not None:
30-
self.update_rules(components, weblog)
36+
self._components: dict[str, Version] = {
37+
name: version for name, version in components.items() if isinstance(version, Version)
38+
}
39+
self.update_rules(self._components, weblog)
3140

3241
def update_rules(
3342
self,
@@ -92,3 +101,43 @@ def format(path: Path = default_manifests_path) -> None:
92101
93102
"""
94103
yml_sort(path)
104+
105+
def assert_versions_not_ahead_of_current(self) -> list[str]:
106+
"""Check that no manifest condition declares a version higher than the current component version.
107+
108+
In dev mode, any version boundary declared in the manifest must not exceed the version currently being tested.
109+
Both component_version and excluded_component_version fields are checked. Prerelease versions of the current
110+
version are not treated as equivalent to the release: 5.2.0-dev is strictly below 5.2.0.
111+
"""
112+
errors = []
113+
114+
for nodeid, conditions in self.data.items():
115+
for condition in conditions:
116+
component = condition["component"]
117+
if component not in self._components:
118+
continue
119+
120+
current_version = self._components[component]
121+
122+
semver_ranges = (
123+
condition.get("component_version"),
124+
condition.get("excluded_component_version"),
125+
)
126+
127+
for sem_range in semver_ranges:
128+
if sem_range is None:
129+
continue
130+
131+
for match in _LOWER_BOUND_RE.finditer(sem_range.expression):
132+
try:
133+
declared = Version(match.group(1))
134+
except (ValueError, TypeError):
135+
continue
136+
137+
if declared > current_version:
138+
errors.append(
139+
f"{nodeid} [{component}]: declared version {declared}"
140+
f" exceeds current version {current_version}"
141+
)
142+
143+
return errors

0 commit comments

Comments
 (0)