Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Speed up tests#977

Merged
PGijsbers merged 15 commits intodevelopopenml/openml-python:developfrom
speed_up_testsopenml/openml-python:speed_up_testsCopy head branch name to clipboard
Oct 29, 2020
Merged

Speed up tests#977
PGijsbers merged 15 commits intodevelopopenml/openml-python:developfrom
speed_up_testsopenml/openml-python:speed_up_testsCopy head branch name to clipboard

Conversation

@PGijsbers
Copy link
Collaborator

A few improvements to tests that should reduce test time without affecting test quality.

The test does not require the list of flows to be updated, to a single
cached version will do fine (this call otherwise would take ~40
seconds).
Downloading a run takes a non-significant amount of time (est. 300ms on
my current setup). It is unnecessary to compare against all >=100 runs,
while a handful should do fine (perhaps even just one should do).
The batch size required in some pages over 40 pages to be loaded, which
increased the workload unnecessarily. These changing preserve pagination
tests while lowering the amount of round trips required.
Since it is already covered by test_run_and_upload_randomsearch.
Speeds up ~25x, and reduces network traffic.
Loading a page takes ~600ms. I don't think testing with 3 pages is any
worse than 10. I also think this is an ideal candidate of test that
could be split up into (1) testing the url is generated correctly, (2)
testing a pre-cached result is parsed correctly and (3) testing the url
gives the expected response (the actual integration test).
If the test is that swapped parameters work, we don't need a complicated pipeline or dataset.
tests/test_evaluations/test_evaluation_functions.py Outdated Show resolved Hide resolved
tests/test_flows/test_flow_functions.py Outdated Show resolved Hide resolved
@PGijsbers PGijsbers requested a review from mfeurer October 27, 2020 19:28
@PGijsbers
Copy link
Collaborator Author

I think this seems like a reasonable set of changes for one PR, so I would like to have some feedback (or approval) 馃憤

Copy link
Collaborator

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

This looks great, but I have once change request. Also, I'd like to see what this does in practice. And finally, there are some failures on appveyor :(

tests/test_runs/test_run_functions.py Outdated Show resolved Hide resolved
@PGijsbers PGijsbers merged commit 07e87ad into develop Oct 29, 2020
@PGijsbers PGijsbers deleted the speed_up_tests branch October 29, 2020 09:49
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.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.