-
Notifications
You must be signed in to change notification settings - Fork 1.6k
tests: defer global imports and remove pytest parametrize decorators #2751
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
base: master
Are you sure you want to change the base?
Conversation
1c3385e to
7d63eca
Compare
7d63eca to
be3b4c6
Compare
0c733c2 to
e5fdd52
Compare
|
I'm only seeing a reduction of 1s to 0.7s collection time with |
07d6e4c to
f0584a7
Compare
|
hello @sshane I was under the impression that we needed to reduce collection time for |
|
as for total test time that seems to be inconsistent on master as well for some reason(locally it takes anywhere between 28sec to 41 secs on my m3 macbook air) |
36b6fb6 to
e926435
Compare
02d155f to
60913c8
Compare
3c61aa1 to
a7b4501
Compare
a7b4501 to
9771d29
Compare
4221971 to
bebb371
Compare
b1a0287 to
6031fe1
Compare
|
Hello again @sshane @adeebshihadeh I’ve refactored some more tests to significantly decrease collection time. It’s not quite <=0.1s yet, but the current PR has already reached the ~500 line limit. It would be best if further optimizations were handled in another PR, and this one reviewed as is. (Getting collection time down further would involve touching all of the values.py files across the codebase, which would be a large refactor, so it's best if that is a seperate pr) |
6a7d6c3 to
a53408f
Compare
|
Hello again @sshane @adeebshihadeh just following up on this pr, did you have a chance to look at it yet? thanks(the benchmarks in pr desc have been updated btw) |
a53408f to
3909f91
Compare
3909f91 to
a0de20b
Compare
relevant issue: #1184
root cause:
Most of the overhead came from heavy import statements, especially files like
opendbc/car/values.py,opendbc/car/interfaces.py,opendbc/car/car_helpers.pyetc. Importing even one of these would trigger a chain of imports, pulling in most of the others. deferring imports from global scope to local scope fixed it.(mostly)I had to remove
parametrizein some places because it required global imports, and replaced it withforloops andsubtests. In the case ofopendbc/car/tests/test_car_interfaces.py, this caused a slowdown since that file has many tests that previously ran in parallel but were now running on a single worker. To fix this, I used a combination of fixtures and batchedparametrizeto get parallelism back.Update: I deffered some more imports in
test_mutation.py,test_ford/hyundai/toyota.I also converted an expensive ffi call from collection time to runtime in