[MNT] Replace live server calls with mocks in test_task_functions.py (partial)#1685
Draft
kronzter wants to merge 2 commits intoopenml:mainfrom
Draft
[MNT] Replace live server calls with mocks in test_task_functions.py (partial)#1685kronzter wants to merge 2 commits intoopenml:mainfrom
kronzter wants to merge 2 commits intoopenml:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Metadata:
Reference Issue: [MNT] Replace Live Server Calls with Mocks in Unit Tests #1649
test_task_functions.pyNew 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.tomlat the repo root, but even then, cache only tests were marked withtest_servermarker.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_typeetc). 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