这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@roelofvandijk
Copy link

@roelofvandijk roelofvandijk commented Mar 6, 2025

Relates to #1184

By moving the car_helpers.py imports to a pytest.fixture, we can shorten the collection time.

Refactors the car tests to use:

I tried to keep the diff as small as possible.

There are 3 big chunks of time spent during the pytest collection on a fresh run (after compilation):

  • first import of CANDefine, aka the packer_pyx compiled library (only slow the first run after compilation)
  • import of the interfaces in car_helpers.py (always slow)
  • import of pytest plugins

"Fresh" denotes a recompilation of the cython modules.

Collect only

pytest opendbc/car/tests --collect-only

State Fresh Second Run
master 0.56s 0.27s
PR 0.16s 0.13s

compare to

pytest opendbc/can/tests --collect-only

State Fresh Second Run
master 0.32s 0.02s
PR 0.29s 0.04s

Collection + execution

pytest opendbc/car/tests

State Fresh Second Run
master 6.42s 6.19s
PR 5.92s 5.76s

@github-actions github-actions bot added the car related to opendbc/car/ label Mar 6, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • include a route or your device' dongle ID if relevant

@roelofvandijk roelofvandijk force-pushed the perf/faster_test_collection branch from a4b587c to e8bfae8 Compare March 6, 2025 19:26
# jerk is measured over half a second
JERK_MEAS_T = 0.5

@pytest.fixture
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this without changing the structure of this test too much?

Copy link
Author

@roelofvandijk roelofvandijk Mar 7, 2025

Choose a reason for hiding this comment

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

If this is to make the review easier, yes. I reduced the diff.

It seems nonsensical, because we are not parametrizing anymore over the test class, but directly over the test functions.

@roelofvandijk roelofvandijk force-pushed the perf/faster_test_collection branch 2 times, most recently from 70d0f30 to 72b8475 Compare March 7, 2025 09:11
@roelofvandijk roelofvandijk force-pushed the perf/faster_test_collection branch from 16e77b7 to 4db39c6 Compare April 30, 2025 08:09
@roelofvandijk roelofvandijk force-pushed the perf/faster_test_collection branch from 4db39c6 to b8fe61b Compare April 30, 2025 08:10
@roelofvandijk
Copy link
Author

@sshane rebased, minimized diff.

@github-actions
Copy link
Contributor

This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity.

@github-actions github-actions bot added the stale label Jun 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2025

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

@github-actions
Copy link
Contributor

This PR has had no activity for 60 days. It will be automatically closed in 7 days if there is no activity.

@github-actions github-actions bot added the stale label Sep 25, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2025

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

@github-actions github-actions bot closed this Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

car related to opendbc/car/ stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants