Skip to content

MNT: Update tests to reflect server side changes - #1765

Merged
PGijsbers merged 5 commits into
mainfrom
update-tests
Oct 9, 2026
Merged

PGijsbers merged 5 commits into
mainfrom
update-tests

Conversation

@PGijsbers

Copy link
Copy Markdown
Collaborator

Metadata

Details

Various fixes to make tests pass.

The REST API has had a few updates after security and availability incidents, and the openml-python tests weren't always updated to reflect those changes. For changes related to #1757 I mostly added a skip instruction, though for some tests it was immediately obvious that even requesting 1000 results (the new limit) would be unnecessary. In those cases I just updated the test.

"contributor, collection_date, language, citation, "
"original_data_url, default_target_attribute, row_id_attribute, "
"ignore_attribute or paper_url to edit.",
)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We used to rely on the server to provide this error response, but since this behavior was even encoded into a unit test, it seems that it should rather be expected of the client to catch the error. (I think it also makes more sense - no need to do a network round trip or force work onto the server)

Comment thread openml/_config.py

def _get_servers(mode: ServerMode) -> dict[APIVersion, dict[str, str | None]]:
if mode not in ServerMode:
if mode not in ServerMode._value2member_map_:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This change was made to support Python 3.10 and 3.11, which is still part of the CI matrix. When we drop support for these Python versions (which should do as soon as possible) we should revert this change.

assert len(attribute_names) == X.shape[1]

@pytest.mark.test_server()
def test_data_edit_non_critical_field(self):

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

After OpenML#1290 we no longer allow editing of any field by users that do not own the dataset, so this test is no longer useful. There already existed a test that asserts the handling of an authorization error when trying to modify a field the user isn't allowed to.

task_ids = [6]
uploader_ids = [1]
flow_ids = [6969]
runs = [6070667, 6070666]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

While the batch limit of 1000 is fine, list_evaluations also has a (server-side) internal result set which is checked, and this was still exceeded. Since we do not care for getting such a large set per se, I filtered the set down to 2 particular runs.

@PGijsbers
PGijsbers requested a review from L-Agbaria October 5, 2026 11:25
@codecov-commenter

codecov-commenter commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.89%. Comparing base (4a25027) to head (434c0f7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1765      +/-   ##
==========================================
- Coverage   81.50%   80.89%   -0.61%     
==========================================
  Files          63       63              
  Lines        5238     5240       +2     
==========================================
- Hits         4269     4239      -30     
- Misses        969     1001      +32     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PGijsbers PGijsbers changed the title Update tests MNT: Update tests to reflect server side changes Oct 5, 2026
@PGijsbers

PGijsbers commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

On the CI results:

  • Failure one is due to an error with the Elastic Search state. I have opened a PR on OpenML to allow us to disable ES, so that we can exclude it from CI in the future.
  • Failure two looks to be a routing issue.
  • Failure three and four are because the services repository needs to be updated to use a newer image of the REST API.
  • Failure five fails on cleanup for unknown reason (to be investigated) should be resolved

@PGijsbers
PGijsbers merged commit 5d7bfc7 into main Oct 9, 2026
25 of 27 checks passed
@PGijsbers
PGijsbers deleted the update-tests branch October 9, 2026 09:46
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.

3 participants