-
Notifications
You must be signed in to change notification settings - Fork 705
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
[SDK] Add UTs for wait_for_job_conditions
#2196
Changes from 5 commits
6214a7c
1ff6ebc
52a4406
53c8e83
db41d3f
2e756d2
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 |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
|
||
import pytest | ||
from kubeflow.training import ( | ||
KubeflowOrgV1JobCondition, | ||
KubeflowOrgV1JobStatus, | ||
KubeflowOrgV1PyTorchJob, | ||
KubeflowOrgV1PyTorchJobSpec, | ||
KubeflowOrgV1ReplicaSpec, | ||
|
@@ -70,6 +72,13 @@ def get(self, timeout): | |
return MockResponse() | ||
|
||
|
||
def get_job_response(*args, **kwargs): | ||
if kwargs.get("namespace") == "runtime": | ||
return generate_job_with_status(create_job(), constants.JOB_CONDITION_FAILED) | ||
else: | ||
return generate_job_with_status(create_job()) | ||
|
||
|
||
def generate_container() -> V1Container: | ||
return V1Container( | ||
name="pytorch", | ||
|
@@ -127,6 +136,21 @@ def create_job(): | |
return pytorchjob | ||
|
||
|
||
def generate_job_with_status( | ||
job: constants.JOB_MODELS_TYPE, | ||
condition_type: str = constants.JOB_CONDITION_SUCCEEDED, | ||
) -> constants.JOB_MODELS_TYPE: | ||
job.status = KubeflowOrgV1JobStatus( | ||
conditions=[ | ||
KubeflowOrgV1JobCondition( | ||
type=condition_type, | ||
status=constants.CONDITION_STATUS_TRUE, | ||
) | ||
] | ||
) | ||
return job | ||
|
||
|
||
class DummyJobClass: | ||
def __init__(self, kind) -> None: | ||
self.kind = kind | ||
|
@@ -279,6 +303,43 @@ def __init__(self, kind) -> None: | |
), | ||
] | ||
|
||
test_data_wait_for_job_conditions = [ | ||
( | ||
"timeout waiting for succeeded condition", | ||
{"name": TEST_NAME, "namespace": "timeout", "wait_timeout": 0}, | ||
TimeoutError, | ||
), | ||
( | ||
"invalid expected condition", | ||
{"name": TEST_NAME, "namespace": "value", "expected_conditions": {"invalid"}}, | ||
ValueError, | ||
), | ||
( | ||
"invalid expected condition(lowercase)", | ||
{"name": TEST_NAME, "namespace": "value", "expected_conditions": {"succeeded"}}, | ||
ValueError, | ||
), | ||
( | ||
"job failed unexpectedly", | ||
{"name": TEST_NAME, "namespace": "runtime"}, | ||
RuntimeError, | ||
), | ||
( | ||
"valid case", | ||
{"name": TEST_NAME, "namespace": "test-namespace"}, | ||
generate_job_with_status(create_job()), | ||
), | ||
( | ||
"valid case with specified callback", | ||
{ | ||
"name": TEST_NAME, | ||
"namespace": "test-namespace", | ||
"callback": lambda job: "test train function", | ||
}, | ||
generate_job_with_status(create_job()), | ||
), | ||
] | ||
|
||
|
||
test_data_get_job_pod_names = [ | ||
( | ||
|
@@ -359,6 +420,15 @@ def training_client(): | |
yield client | ||
|
||
|
||
@pytest.fixture | ||
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. Do you need separate fixture just for wait_for_job_condition or we can re-use Training client like for other tests ? 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. I separate fixture for |
||
def training_client_wait_for_job_conditions(): | ||
with patch.object(TrainingClient, "get_job", side_effect=get_job_response), patch( | ||
"kubernetes.config.load_kube_config", return_value=Mock() | ||
): | ||
client = TrainingClient(job_kind=constants.PYTORCHJOB_KIND) | ||
yield client | ||
|
||
|
||
@pytest.mark.parametrize("test_name,kwargs,expected_output", test_data_create_job) | ||
def test_create_job(training_client, test_name, kwargs, expected_output): | ||
""" | ||
|
@@ -434,3 +504,21 @@ def test_update_job(training_client, test_name, kwargs, expected_output): | |
except Exception as e: | ||
assert type(e) is expected_output | ||
print("test execution complete") | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"test_name,kwargs,expected_output", test_data_wait_for_job_conditions | ||
) | ||
def test_wait_for_job_conditions( | ||
training_client_wait_for_job_conditions, test_name, kwargs, expected_output | ||
): | ||
""" | ||
test wait_for_job_conditions function of training client | ||
""" | ||
print("Executing test:", test_name) | ||
try: | ||
out = training_client_wait_for_job_conditions.wait_for_job_conditions(**kwargs) | ||
assert out == expected_output | ||
except Exception as e: | ||
assert type(e) is expected_output | ||
print("test execution complete") |
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.
You can use timeout namespace from consts: https://github.com/kubeflow/training-operator/blob/db41d3f05090387e9bf7a57375bbc20303bcf5b2/sdk/python/kubeflow/training/api/training_client_test.py#L26C1-L26C8
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.
Okay, I'll replace that.