TST mock download_subject test to improve flakyness#640
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 600-second timeout for the test_download_subject test in cortex/tests/test_utils.py. Feedback indicates that this duration is excessively long for a single test and suggests refactoring the implementation to avoid redundant downloads or using a mock subject to improve CI efficiency.
| import cortex | ||
| import pytest | ||
|
|
||
| @pytest.mark.timeout(600) |
There was a problem hiding this comment.
A timeout of 600 seconds (10 minutes) is exceptionally long for a single test and can significantly delay CI feedback loops. This duration is largely driven by the fact that the test downloads the 'fsaverage' subject twice (once on line 13 and again on line 16 with download_again=True).
To improve efficiency and reduce the risk of hitting timeouts, consider refactoring the test to:
- Download the subject only once.
- Use a smaller mock subject if the goal is specifically to test the download mechanism.
- Mark the test with
@pytest.mark.slowor a custom network marker so it can be optionally skipped during rapid local development.
Replaces the real fsaverage download with a fake in-memory tarball and an isolated filestore, so tests no longer race on download bandwidth or time out under concurrent CI runs. Adds coverage for the skip-when-present and download_again=True branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mvdoc
changed the title
1 participant
Downloading the test subject may take some time