Skip to content

Commit 0180184

Browse files
author
Nissan Pow
committed
test: add unit tests for @resources vs @kubernetes precedence
6 tests covering the merge logic when both decorators specify memory/cpu: default override, explicit max, both-at-default.
1 parent bf96ce9 commit 0180184

1 file changed

Lines changed: 58 additions & 34 deletions

File tree

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,78 @@
1-
"""Tests for @resources vs @kubernetes decorator precedence logic.
1+
"""Tests for @resources vs @kubernetes precedence logic.
22
3-
The merge logic in KubernetesDecorator.step_init works as follows:
4-
- If @kubernetes attribute is at its default AND @resources explicitly sets a value
5-
→ use the @resources value (even if lower than the @kubernetes default).
6-
- If both explicitly set → take max.
7-
- If both at default → take max (which equals default).
3+
The merge logic in KubernetesDecorator.step_init determines which value
4+
wins when both @resources and @kubernetes set the same attribute (e.g. memory).
5+
6+
Rules:
7+
- If @kubernetes is at its default and @resources explicitly sets a value,
8+
@resources wins (even if lower).
9+
- If both are explicitly set, the larger value wins.
10+
- If both are at default, the default is kept.
811
"""
912

10-
K8S_DEFAULTS = {"cpu": "1", "memory": "4096", "disk": "10240"}
11-
RESOURCES_DEFAULTS = {"cpu": "1", "memory": "4096"}
13+
from metaflow.plugins.kubernetes.kubernetes_decorator import KubernetesDecorator
14+
from metaflow.plugins.resources_decorator import ResourcesDecorator
15+
16+
17+
def _merge_resource(k8s_val, resources_val, attr="memory"):
18+
"""Simulate the merge logic from step_init for a single attribute.
19+
20+
Returns the resolved value as a string (matching self.attributes format).
21+
"""
22+
k8s_default = KubernetesDecorator.defaults[attr]
23+
res_default = ResourcesDecorator.defaults.get(attr)
1224

25+
my_val = k8s_val
26+
v = resources_val
1327

14-
def _merge(k, k8s_val, resources_val):
15-
"""Replicate the merge logic from KubernetesDecorator.step_init."""
16-
k8s_is_default = float(k8s_val or 0) == float(K8S_DEFAULTS.get(k) or 0)
17-
resources_is_default = k in RESOURCES_DEFAULTS and float(
18-
resources_val or 0
19-
) == float(RESOURCES_DEFAULTS.get(k) or 0)
28+
if my_val is None and v is None:
29+
return str(my_val)
30+
31+
k8s_is_default = float(my_val or 0) == float(k8s_default or 0)
32+
resources_is_default = attr in ResourcesDecorator.defaults and float(
33+
v or 0
34+
) == float(res_default or 0)
2035

2136
if k8s_is_default and not resources_is_default:
22-
return str(float(resources_val or 0))
37+
return str(float(v or 0))
2338
else:
24-
return str(max(float(k8s_val or 0), float(resources_val or 0)))
39+
return str(max(float(my_val or 0), float(v or 0)))
2540

2641

27-
def test_resources_lower_than_k8s_default():
28-
"""@resources(memory=256) with @kubernetes default (4096) → resources wins."""
29-
result = _merge("memory", K8S_DEFAULTS["memory"], "256")
30-
assert result == "256.0"
42+
def test_resources_lower_than_k8s_default_wins():
43+
"""@resources(memory=256) should override @kubernetes default (4096)."""
44+
result = _merge_resource("4096", "256")
45+
assert float(result) == 256.0
3146

3247

33-
def test_resources_higher_than_k8s_default():
34-
"""@resources(memory=8192) with @kubernetes default (4096) → resources wins."""
35-
result = _merge("memory", K8S_DEFAULTS["memory"], "8192")
36-
assert result == "8192.0"
48+
def test_resources_higher_than_k8s_default_wins():
49+
"""@resources(memory=8192) overrides @kubernetes default (4096)."""
50+
result = _merge_resource("4096", "8192")
51+
assert float(result) == 8192.0
3752

3853

39-
def test_both_explicit_resources_higher():
40-
"""@kubernetes(memory=2048) explicit with @resources(memory=8192) → max wins."""
41-
result = _merge("memory", "2048", "8192")
42-
assert result == "8192.0"
54+
def test_both_explicit_takes_max():
55+
"""@kubernetes(memory=2048) + @resources(memory=8192) → max wins."""
56+
result = _merge_resource("2048", "8192")
57+
# k8s is not at default (2048 != 4096), so max(2048, 8192) = 8192
58+
assert float(result) == 8192.0
4359

4460

4561
def test_both_explicit_k8s_higher():
46-
"""@kubernetes(memory=8192) explicit with @resources(memory=2048) → max wins."""
47-
result = _merge("memory", "8192", "2048")
48-
assert result == "8192.0"
62+
"""@kubernetes(memory=8192) + @resources(memory=2048) → k8s wins."""
63+
result = _merge_resource("8192", "2048")
64+
assert float(result) == 8192.0
4965

5066

5167
def test_both_at_default():
52-
"""Both at default → stays at default (4096)."""
53-
result = _merge("memory", K8S_DEFAULTS["memory"], RESOURCES_DEFAULTS["memory"])
54-
assert result == "4096.0"
68+
"""Both at default → stays at default."""
69+
k8s_default = KubernetesDecorator.defaults["memory"]
70+
res_default = ResourcesDecorator.defaults["memory"]
71+
result = _merge_resource(k8s_default, res_default)
72+
assert float(result) == float(k8s_default)
73+
74+
75+
def test_cpu_resources_overrides_k8s_default():
76+
"""@resources(cpu=2) should override @kubernetes default cpu (1)."""
77+
result = _merge_resource("1", "2", attr="cpu")
78+
assert float(result) == 2.0

0 commit comments

Comments
 (0)