Skip to content

replace live server calls in tests/test_tasks/test_task.py #1683

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.

Sign up for GitHub

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

Open
omkar-334 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

replace live server calls in tests/test_tasks/test_task.py #1683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/files/mock_responses/tasks/task_upload.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<oml:upload_task xmlns:oml="http://openml.org/openml">
<oml:id>99999</oml:id>
</oml:upload_task>
78 changes: 44 additions & 34 deletions tests/test_tasks/test_task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@

import unittest
from random import randint, shuffle
from unittest import mock

import pytest
import requests

from openml.datasets import (
get_dataset,
list_datasets,
)
from openml.exceptions import OpenMLServerException
from openml.tasks import TaskType, create_task, get_task
from openml.testing import TestBase
from openml.testing import TestBase, create_request_response


class OpenMLTaskTest(TestBase):
Expand All @@ -32,47 +34,55 @@ def setUpClass(cls):
def setUp(self, n_levels: int = 1):
super().setUp()

@pytest.mark.test_server()
def test_download_task(self):
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test_download_task method still makes a live server call through get_task but no longer has the @pytest.mark.test_server decorator. This test needs to be mocked to avoid hitting the live server. Consider mocking requests.Session.get to provide a mock task XML response.

Suggested change
def test_download_task(self):
@mock.patch.object(requests.Session, "get")
def test_download_task(self, mock_get):
tasks_dir = self.static_cache_dir / "mock_responses" / "tasks"
mock_get.return_value = create_request_response(
status_code=200, content_filepath=tasks_dir / "task.xml"
)

Copilot uses AI. Check for mistakes.
return get_task(self.task_id)

@pytest.mark.test_server()
def test_upload_task(self):
@mock.patch.object(requests.Session, "post")
def test_upload_task(self, mock_post):
tasks_dir = self.static_cache_dir / "mock_responses" / "tasks"
mock_post.side_effect = [
create_request_response(
status_code=200, content_filepath=tasks_dir / "task_upload.xml"
),
]

# We don't know if the task in question already exists, so we try a few times. Checking
# beforehand would not be an option because a concurrent unit test could potentially
# create the same task and make this unit test fail (i.e. getting a dataset and creating
Comment on lines +43 to 51
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock_post.side_effect provides only one response, but the loop can iterate up to 100 times (lines 56-85), each calling task.publish() which makes a POST request. After the first iteration, subsequent calls to mock_post will raise a StopIteration error because side_effect has been exhausted. Either simplify the test to run once (recommended since mocking eliminates the need for retries), or provide 100 mock responses if the retry logic must be tested.

Suggested change
mock_post.side_effect = [
create_request_response(
status_code=200, content_filepath=tasks_dir / "task_upload.xml"
),
]
# We don't know if the task in question already exists, so we try a few times. Checking
# beforehand would not be an option because a concurrent unit test could potentially
# create the same task and make this unit test fail (i.e. getting a dataset and creating
mock_post.return_value = create_request_response(
status_code=200, content_filepath=tasks_dir / "task_upload.xml"
)
# We don't know if the task in question already exists, so we try a few times. Checking
# beforehand would not be an option because a concurrent unit test could potentially
# create the same task and make this unit test fail (i.e. getting a dataset and creating
# beforehand would not be an option because a concurrent unit test could potentially
# create the same task and make this unit test fail (i.e. getting a dataset and creating

Copilot uses AI. Check for mistakes.
# a task for it is not atomic).
Comment on lines 49 to 52
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment mentions "Checking beforehand would not be an option because a concurrent unit test could potentially create the same task" which refers to race conditions on the live server. With mocking, there are no concurrent tests accessing a shared server state, so this comment is no longer accurate and should be updated or removed to reflect that the test now uses mocks.

Suggested change
# We don't know if the task in question already exists, so we try a few times. Checking
# beforehand would not be an option because a concurrent unit test could potentially
# create the same task and make this unit test fail (i.e. getting a dataset and creating
# a task for it is not atomic).
# We don't know if the task in question already exists on the server, so we try creating
# it multiple times and handle the "task already exists" error (code 614) as it would
# occur in real-world usage. This test simulates that behavior using mocked responses.

Copilot uses AI. Check for mistakes.
compatible_datasets = self._get_compatible_rand_dataset()
for i in range(100):
try:
dataset_id = compatible_datasets[i % len(compatible_datasets)]
# TODO consider implementing on the diff task types.
task = create_task(
task_type=self.task_type,
dataset_id=dataset_id,
target_name=self._get_random_feature(dataset_id),
estimation_procedure_id=self.estimation_procedure,
)

task.publish()
TestBase._mark_entity_for_removal("task", task.id)
TestBase.logger.info(
f"collected from {__file__.split('/')[-1]}: {task.id}",
with mock.patch.object(OpenMLTaskTest, "_get_compatible_rand_dataset", return_value=[20]), \
mock.patch.object(OpenMLTaskTest, "_get_random_feature", return_value="class"):
Comment on lines +53 to +54
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mocked methods _get_compatible_rand_dataset and _get_random_feature bypass the original logic, but the actual methods at lines 87-122 still call list_datasets and get_dataset, which make server calls. If any subclass or test directly calls these helper methods, they will hit the live server. Consider adding mocks for requests.Session.get to mock the responses from list_datasets and get_dataset to make this test truly isolated.

Copilot uses AI. Check for mistakes.
compatible_datasets = self._get_compatible_rand_dataset()
for i in range(100):
try:
dataset_id = compatible_datasets[i % len(compatible_datasets)]
# TODO consider implementing on the diff task types.
task = create_task(
task_type=self.task_type,
dataset_id=dataset_id,
target_name=self._get_random_feature(dataset_id),
estimation_procedure_id=self.estimation_procedure,
)

task.publish()
TestBase._mark_entity_for_removal("task", task.id)
TestBase.logger.info(
f"collected from {__file__.split('/')[-1]}: {task.id}",
)
# success
break
except OpenMLServerException as e:
# Error code for 'task already exists'
# Should be 533 according to the docs
# (# https://www.openml.org/api_docs#!/task/post_task)
if e.code == 614:
continue
else:
raise e
Comment on lines +74 to +81
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception handling for OpenMLServerException with error code 614 ("task already exists") is designed for live server interactions where tasks might already exist. With mocking, the mock response is predetermined and won't raise this exception unless explicitly configured to do so. This exception handling is now dead code and can be removed when the test is simplified to run once with the mock.

Copilot uses AI. Check for mistakes.
else:
raise ValueError(
f"Could not create a valid task for task type ID {self.task_type}",
)
Comment on lines +56 to 85
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With mocking in place, the 100-iteration loop is no longer necessary because the server errors (code 614) won't occur with a mocked response. The test could be simplified to execute only once, which would make it faster and clearer. The retry logic was designed to handle race conditions on the live server, which don't exist when using mocks.

Copilot uses AI. Check for mistakes.
# success
break
except OpenMLServerException as e:
# Error code for 'task already exists'
# Should be 533 according to the docs
# (# https://www.openml.org/api_docs#!/task/post_task)
if e.code == 614:
continue
else:
raise e
else:
raise ValueError(
f"Could not create a valid task for task type ID {self.task_type}",
)

def _get_compatible_rand_dataset(self) -> list:
active_datasets = list_datasets(status="active")
Expand Down
Loading