-
Notifications
You must be signed in to change notification settings - Fork 50
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 unit tests #1821
Comments
There definitely are some tests that just take longer but that still need to be tested. What we could do is mark some tests as slow ( At some point we should maybe also think about refactoring the tests... |
Actually that's strange. I named that file I'm checking now how to avoid it being included, but running into a weird |
In our [tool.pytest.ini_options]
python_files = "*.py" Meaning, we are globbing for any python file, and the test is then discovered due to it being named |
Oh yeah, that was a doctest change. Nice, we should move it to |
Alternatively, we could mark it, by registering a marker in [pytest]
markers =
optional: mark a test as optional. Mark the test: @pytest.mark.optional
def test_import():
... And then we can exclude this marker from running by default and run it via |
Maybe let's just have a tiny PR for that so it already saves ~3mins for the tests? :) |
PR opened with the move! |
To add to this issue. Yannick suggested also to improve what we're caching in the Github actions. I think we can cache the poetry virtual environment but this did cause issues beforehand during the first poetry implementation, but I think can be revisited. |
I can leave this open for future improvements, but I think nothing else is planned for now? |
Our tests are starting to get a bit heavy to run, so I timed them. Here's all that take more than 1s.
The
tidy3d_import.py
one imports 100 times, which seems excessive. It also doesn't really test anything, it only prints. Was it not supposed to be part of the test suite at all @daquintero, rather just for internal use?I made a PR for the
test_num_mediums
one.Apart from that there's a lot of adjoint/autograd stuff @tylerflex @yaugenst-flex , maybe some of it is inevitable, but maybe some can be improved? Not high priority.
Then there's some simulation data and mode solver ones, but again, not high priority.
The text was updated successfully, but these errors were encountered: