Skip to content

Commit 1e9f658

Browse files
talsperreclaude
andcommitted
test: slim _graph_endpoints unit tests, drop redundant happy-path checks
The happy-path coverage (custom-named-step flows producing the right endpoints) lives in test/ux/core/test_basic.py, which runs real flows via the Runner / scheduler deployer chain across local and scheduler modes. Three of the unit tests in this file were just re-asserting that contract without exercising the metadata layer end-to-end. Drop: - test_metadata_carries_endpoints (covered by test_custom_step_names) - test_partial_metadata_fills_in_defaults (edge case, dict.get semantics) - test_result_cached (cache write is implicitly verified by test_metaflow_not_found_caches_fallback) Replace the make_run factory fixture and the __class__ swap trick with mocker.patch.object(Run, "__getitem__", ...) on a Run.__new__(Run) instance. This is the pytest-mock idiom now codified in CONTRIBUTING.md and matches how other unit tests will look going forward. Three tests remain, each pinning a non-obvious behavior: - empty metadata falls back to ("start", "end") - MetaflowNotFound (old run) caches the fallback - transient exceptions do NOT cache (so a retry can succeed) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0e95d13 commit 1e9f658

1 file changed

Lines changed: 30 additions & 86 deletions

File tree

Lines changed: 30 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
"""Tests for Run._graph_endpoints fallback behavior.
1+
"""Tests for Run._graph_endpoints fallback and caching behavior.
22
3-
``_parameters`` task metadata is written by ``persist_constants``, which
4-
every runtime path calls before any step runs (native runtime directly, and
5-
orchestrators via the ``init`` command they insert). The property reads
6-
``start_step``/``end_step`` from that metadata with a literal
7-
``("start", "end")`` fallback for old runs.
3+
The happy path (custom-named steps producing the right endpoints) is
4+
covered by the integration tests in test/ux/core/test_basic.py
5+
(test_custom_step_names, test_single_step_flow, test_custom_branch_flow).
6+
This file pins the smaller, harder-to-reach behaviors:
87
9-
These tests verify the lookup and that transient errors are not cached.
8+
- empty / missing metadata returns the legacy ("start", "end") fallback
9+
- transient errors do NOT cache (so a retry can succeed)
10+
- MetaflowNotFound (old run) DOES cache (no point retrying forever)
1011
"""
1112

1213
import pytest
@@ -16,90 +17,40 @@
1617

1718

1819
@pytest.fixture
19-
def make_run(mocker):
20-
"""Factory fixture: build a Run-like object whose ``__getitem__``
21-
returns a task with a controlled ``metadata_dict`` (or raises)."""
20+
def run():
21+
"""Bare Run instance, Run.__init__ skipped to avoid metadata service I/O."""
22+
return Run.__new__(Run)
2223

23-
def _factory(metadata_dict=None, raises=None):
24-
run = Run.__new__(Run) # bypass __init__
2524

26-
if raises is not None:
25+
def _params_step(mocker, metadata):
26+
"""Stand-in for run["_parameters"] with a controlled metadata_dict."""
27+
params = mocker.MagicMock()
28+
params.task.metadata_dict = metadata
29+
return params
2730

28-
def _getitem(key):
29-
raise raises
3031

31-
else:
32-
task = mocker.MagicMock()
33-
task.metadata_dict = metadata_dict or {}
34-
params_step = mocker.MagicMock()
35-
params_step.task = task
36-
37-
def _getitem(key):
38-
if key == "_parameters":
39-
return params_step
40-
raise KeyError(key)
41-
42-
run.__class__ = type(
43-
"FakeRun",
44-
(object,),
45-
{
46-
"__getitem__": lambda self, k: _getitem(k),
47-
"_graph_endpoints": Run._graph_endpoints,
48-
},
49-
)
50-
return run
51-
52-
return _factory
53-
54-
55-
def test_metadata_carries_endpoints(make_run):
56-
"""Metadata has start_step/end_step, used directly."""
57-
run = make_run({"start_step": "begin", "end_step": "finish"})
58-
assert run._graph_endpoints == ("begin", "finish")
59-
60-
61-
def test_missing_metadata_falls_back_to_literals(make_run):
62-
"""Empty metadata, fall back to ('start', 'end')."""
63-
run = make_run({})
32+
def test_missing_metadata_falls_back_to_literals(run, mocker):
33+
"""Empty metadata returns ('start', 'end')."""
34+
mocker.patch.object(Run, "__getitem__", return_value=_params_step(mocker, {}))
6435
assert run._graph_endpoints == ("start", "end")
6536

6637

67-
def test_partial_metadata_fills_in_defaults(make_run):
68-
"""Only one endpoint in metadata, the other defaults to its literal."""
69-
run = make_run({"start_step": "begin"})
70-
assert run._graph_endpoints == ("begin", "end")
71-
72-
73-
def test_result_cached(make_run):
74-
"""Successful lookups cache on _cached_endpoints."""
75-
run = make_run({"start_step": "begin", "end_step": "finish"})
76-
assert run._graph_endpoints == ("begin", "finish")
77-
assert run._cached_endpoints == ("begin", "finish")
38+
def test_metaflow_not_found_caches_fallback(run, mocker):
39+
"""MetaflowNotFound (old run, no _parameters) caches the fallback."""
40+
mocker.patch.object(Run, "__getitem__", side_effect=MetaflowNotFound("_parameters"))
41+
assert run._graph_endpoints == ("start", "end")
42+
assert run._cached_endpoints == ("start", "end")
7843

7944

80-
def test_transient_error_not_cached(mocker):
45+
def test_transient_error_not_cached(run, mocker):
8146
"""A transient exception returns the fallback but does NOT cache it."""
82-
run = Run.__new__(Run)
83-
84-
task = mocker.MagicMock()
85-
task.metadata_dict = {"start_step": "begin", "end_step": "finish"}
86-
params_step = mocker.MagicMock()
87-
params_step.task = task
88-
89-
getitem = mocker.MagicMock(
47+
mocker.patch.object(
48+
Run,
49+
"__getitem__",
9050
side_effect=[
9151
RuntimeError("transient (e.g., metadata service down)"),
92-
params_step,
93-
]
94-
)
95-
96-
run.__class__ = type(
97-
"FakeRun",
98-
(object,),
99-
{
100-
"__getitem__": lambda self, k: getitem(k),
101-
"_graph_endpoints": Run._graph_endpoints,
102-
},
52+
_params_step(mocker, {"start_step": "begin", "end_step": "finish"}),
53+
],
10354
)
10455

10556
# First call: transient error, fallback returned, not cached.
@@ -109,10 +60,3 @@ def test_transient_error_not_cached(mocker):
10960
# Second call: succeeds, caches.
11061
assert run._graph_endpoints == ("begin", "finish")
11162
assert run._cached_endpoints == ("begin", "finish")
112-
113-
114-
def test_metaflow_not_found_cached(make_run):
115-
"""MetaflowNotFound (old run) caches the ('start', 'end') fallback."""
116-
run = make_run(raises=MetaflowNotFound("_parameters"))
117-
assert run._graph_endpoints == ("start", "end")
118-
assert run._cached_endpoints == ("start", "end")

0 commit comments

Comments
 (0)