Speed up unit tests. Closes #2958#3289
Conversation
|
on my machine: 16m->4m |
IntelOwl/api_app/analyzers_manager/file_analyzers/capa_info.py Lines 52 to 81 in 8419ace Shouldnt this method be patched out, my commit has nothing to do with this, but its still failing |
|
IntelOwl/tests/api_app/analyzers_manager/test_views.py Lines 32 to 39 in 8556c53 what is this test even doing? i dont think its supposed to call /pull twice, i'll just bandage this for now |
|
https://github.com/intelowlproject/IntelOwl/blob/develop/tests/test_crons.py#L245-L247 |
|
the test step of the ci now takes ~ half time |
There was a problem hiding this comment.
Pull request overview
This PR focuses on reducing CI/unit-test runtime by preventing slow external operations (Celery task scheduling, polling sleeps, and updater downloads/git activity) from running during tests.
Changes:
- Skip or mock slow updater behavior (e.g., Yara updater in cron tests; Capa signatures download).
- Patch Celery pipeline enqueue (
job_pipeline.apply_async) during API tests to avoid running the full job pipeline. - Patch
time.sleepin analyzer unit tests to eliminate real polling delays.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_crons.py |
Skips the Yara updater test under MOCK_CONNECTIONS to avoid unmocked git operations. |
tests/api_app/test_api.py |
Patches job_pipeline.apply_async for several endpoints/rescan tests to avoid slow pipeline execution. |
tests/api_app/analyzers_manager/unit_tests/observable_analyzers/test_pulsedive.py |
Patches time.sleep to remove polling delay. |
tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_virushee.py |
Patches time.sleep to remove polling delay. |
tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_capa_info.py |
Mocks signature download to avoid filesystem/network work during tests. |
tests/api_app/analyzers_manager/test_views.py |
Wraps analyzer “pull” calls with a mocked YaraScan.update to avoid slow updater execution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_capa_info.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/api_app/test_api.py
Outdated
| }, | ||
| ) | ||
| response = self.client.post(f"/api/jobs/{job.pk}/rescan", format="json") | ||
| # dont actually run the analyzers, they are tested in unit tests |
There was a problem hiding this comment.
Typo in comment: "dont" → "don't".
| # dont actually run the analyzers, they are tested in unit tests | |
| # don't actually run the analyzers, they are tested in unit tests |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_crons.py
Outdated
| mock_repo.clone_from.side_effect = lambda url, path, **kwargs: os.makedirs(path, exist_ok=True) | ||
| # When settings.MOCK_CONNECTIONS is False, @if_mock_connections is a no-op | ||
| # and no mocks are injected, so mock_zipfile will be None. In that case, | ||
| # skip this test to avoid AttributeError in CI. | ||
| if mock_zipfile is None: | ||
| self.skipTest("MOCK_CONNECTIONS is disabled; yara updater test requires mocked connections.") | ||
| mock_zipfile.return_value.extractall.side_effect = lambda path: os.makedirs(path, exist_ok=True) |
There was a problem hiding this comment.
These mocks only create directories, but YaraScan.update() calls repo.compile() for each configured repository. With no .yar/.yara/.rule files created by the mocks, YaraRepo.compile() will attempt to compile an empty ruleset and is likely to error. To keep this fast and deterministic, either mock out YaraRepo.compile/yara.compile for this test, or have the mock extractall/clone_from create at least one minimal valid YARA rule file in the repo directory.
| mock_repo.clone_from.side_effect = lambda url, path, **kwargs: os.makedirs(path, exist_ok=True) | |
| # When settings.MOCK_CONNECTIONS is False, @if_mock_connections is a no-op | |
| # and no mocks are injected, so mock_zipfile will be None. In that case, | |
| # skip this test to avoid AttributeError in CI. | |
| if mock_zipfile is None: | |
| self.skipTest("MOCK_CONNECTIONS is disabled; yara updater test requires mocked connections.") | |
| mock_zipfile.return_value.extractall.side_effect = lambda path: os.makedirs(path, exist_ok=True) | |
| def _create_dummy_rule(path: str) -> None: | |
| os.makedirs(path, exist_ok=True) | |
| rule_path = os.path.join(path, "dummy.yar") | |
| # Minimal valid YARA rule to avoid empty ruleset compilation errors | |
| with open(rule_path, "w", encoding="utf-8") as f: | |
| f.write("rule dummy { condition: true }") | |
| mock_repo.clone_from.side_effect = ( | |
| lambda url, path, **kwargs: _create_dummy_rule(path) | |
| ) | |
| # When settings.MOCK_CONNECTIONS is False, @if_mock_connections is a no-op | |
| # and no mocks are injected, so mock_zipfile will be None. In that case, | |
| # skip this test to avoid AttributeError in CI. | |
| if mock_zipfile is None: | |
| self.skipTest("MOCK_CONNECTIONS is disabled; yara updater test requires mocked connections.") | |
| mock_zipfile.return_value.extractall.side_effect = ( | |
| lambda path: _create_dummy_rule(path) | |
| ) |
|
Is there anything else you'd like me to do? |
|
if you can please address the remaining copilot issues |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I don't think the second change is required, compile method would've failed the test if that was the behaviour, but you can commit the suggestion if youd like |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_virushee.py
Outdated
Show resolved
Hide resolved
tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_capa_info.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Speed up unit tests. Closes #2958
Type of change
Checklist
develop# This file is a part of IntelOwl https://github.com/intelowlproject/IntelOwl # See the file 'LICENSE' for copying permission.Ruff) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.testsfolder). All the tests (new and old ones) gave 0 errors.DeepSource,Django Doctorsor other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.