Skip to content

Comments

[MNT] Replace live server calls with mocks in test_task_functions.py (partial)#1685

Draft
kronzter wants to merge 2 commits intoopenml:mainfrom
kronzter:maint/mock-test-task-functions
Draft

[MNT] Replace live server calls with mocks in test_task_functions.py (partial)#1685
kronzter wants to merge 2 commits intoopenml:mainfrom
kronzter:maint/mock-test-task-functions

Conversation

@kronzter
Copy link

Metadata:

  • Reference Issue: [MNT] Replace Live Server Calls with Mocks in Unit Tests #1649 test_task_functions.py

  • New Tests Added: No

  • Documentation Updated: NA

  • Change Log Entry: please read below

  • What does this PR implement/fix? Explain your changes.*
    Implemented by storing server response xml directly into a mock response directory, and instead of making a http request, checking locally.

  • Why is this change necessary? What is the problem it solves?*
    Yes, reduces server test running duration.

  • Any other comments?* Yes
    1 A few days ago, as per a merged PR, cache-only tests were given test_server (or production_server) markers. There is already a provision for cache-related markers in pyproject.toml at the repo root, but even then, cache only tests were marked with test_server marker.
    I dont think that was intended. Should i remove markers accordingly or mark with cache marker? I have marked a few with cache marker.

2 Most of the more obvious candidates have been converted to mocks (e.g. test__get_estimation_procedure_list, test_list_clustering_task, test_list_tasks_by_type etc). For the rest, there is some uncertainty about what was intentionally left server-dependent vs. what was not. To make that explicit, every test that was not converted is marked in the file with a # Intentionally left unmocked (*number) comment and (*number) as identifier, and the test names below are those left unmocked:
test_list_tasks_empty — would require client changes for empty/error response handling.
test_list_tasks — requires a very large fixture (≥900 tasks).
test_list_tasks_paginate — multiple sequential GETs.
test_list_tasks_per_type_paginate — nested loop, many GETs.
test__get_task_live — already skipped; left as-is.
test_get_task — asserts on filesystem after real download.
test_removal_upon_download_failure — partially mocked previously;
test_get_task_different_types — three GETs; could be mocked in a follow-up.
test_download_split — needs real split ARFF download and file on disk.

If any maintainers can share their valuable insights as to what was intended and what wasn't, it would be much appreciated.

work in progress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant