Repository navigation
[MNT] Add Missing Assertions to Run Function Tests #1646
Description
Several tests in test_run_functions.py have TODO comments indicating missing assertions (e.g., lines 170, 287, 291, 341, 514, 555). These include checks for holdout tasks, parameter presence, and runtime values, which would improve test reliability.
Steps:
- Review each TODO comment and the surrounding test code to understand what's being tested.
- Add appropriate assertions, such as:
assert task.task_type == "holdout"for holdout task checks.assert "runtime" in run.attributesfor runtime presence.assert len(run.parameters) > 0for parameter checks.
- Run the tests to ensure they pass and cover the intended behavior.
- If needed, mock or set up test data to make assertions deterministic.
Tests will be more comprehensive, catching regressions in run handling and parameter validation.
Activity
vanshg101 commentedon Feb 18, 2026
The fix seems straightforward. The TODOs are mostly just missing assert statements after code that already runs correctly. For the holdout check (line 170), we just need to fetch the task and verify the estimation procedure type. The XML/ARFF ones (287, 291) are literally just capturing the return values and checking theyare not empty/have the right keys... The runtime one (555) is the same idea assert the keys exist before the timing helper tries to use them...
Line 514 I'd leave alone, that's a refactoring suggestion not a missing assertion.
Raising a PR with this approach!
Abhishek9639 commentedon Mar 7, 2026
Hi @geetu040,
I would like to work on this issue.
I’ll go through the TODO comments in "test_run_functions.py" and add the appropriate assertions as suggested.
Please let me know if there is anything specific I should keep in mind.
Thanks
Address issue openml#1646: Add missing assertions to run function tests
Pg1910 commentedon Mar 8, 2026
I have read the code and these were my intuitions :-
line(170) -Here,the clean assertion is to verify that the referenced run belongs to a holdout task before comparing predictions.
line(287)and (291) - for run._to_xml(),it actually returns a non-empty XML string ,for run.trace.trace_to_arff(),it returns a valid ARFF-style dictionary. these are just smoke . for assertion we can check serialization as well.
line(341) -- compare the local trace with the downloaded trace fetched explicitly from the trace endpoint.
line(514) -- here we need to add a second deterministic test and keep the original one as backend integration.
opening a PR with my approach/inference .
Aishu-ks commentedon Aug 12, 2026
Hi! I'd like to work on this issue. I plan to add the missing assertions to the affected tests and run the relevant test suite to verify the changes.
rishabhkumaryadav commentedon Sep 5, 2026
/claim
P0llyanna commentedon Sep 14, 2026
Hi! I’d like to work on this issue as my first contribution to openml-python. I’ll take a look at the missing assertions and add the corresponding tests.
Bhoomika910 commentedon Sep 17, 2026
Hi, I'd like to work on this issue. I will review the TODOs in test_run_functions.py, add the appropriate assertions, and run the relevant tests.
Gaurav-Kanse commentedon Sep 26, 2026
Hi! I'd like to work on this issue. I can take care of adding the missing assertions and updating the relevant tests.
AtharvaManale commentedon Oct 1, 2026
I’ve implemented a subset of the missing assertions from this issue and opened a PR for review.
The changes currently cover:
I’ve intentionally left the other TODOs in test_run_functions.py unchanged, including the server-dependent initialize_model_from_trace mocking TODO and the runtime check in fold_evaluations has been already validated using _check_fold_timing_evaluations functions
PR: #1758