Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up tests #977

Merged
merged 15 commits into from
Oct 29, 2020
Merged

Speed up tests #977

merged 15 commits into from
Oct 29, 2020

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.
@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